Re: [RFA] Linker script extension SECTION_FLAGS

[Tristan Gingold <gingold at adacore dot com>]

On May 25, 2011, at 12:57 AM, Catherine Moore wrote:
> 
> A new patch that addresses these concerns plus a couple that were pointed out by Tristan is attached here.  Tristan, I didn't incorporate your comment regarding the backend hook into the new patch.  I have now filled in bfd_elf_lookup_section_flags to handle all of the ELF common section header flags.

Ok, that makes sense.

>  The backend hook is intended to handle the target-specific flags.  Although, I'm not submitting it at this time, the backend hook that I am using for the PowerPC VLE target is attached as an example.
> 
> Please let me know if there are additional concerns or comments and whether this is okay to commit.

I am not an ELF maintainer, but still I'd like to make three additional comments:

* I think you'd better to use a for statement for this pattern (in ppc_elf_lookup_section_flags and bfd_elf_lookup_section_flags):

+  struct flag_info_list *tf = finfo->flag_list;
..
+
+  while (tf)
+    {
..
+      tf = tf->next;
+    }

Something like:
  for (tf = finfo->flag_list; tf != NULL; tf = tf->next)
clarify the iteration use.

* bfd_generic_lookup_section_flags shouldn't silently accept any flag, but generates an error message.

* I think you should simplify the bfd_lookup_section_flags hook: it should only convert a name to a flag and return an error/success
  status.  All the logical operations (setting only_with_flag or not_with_flag) and error handling should be made by the caller.
  You could add a new function, bfd_lookup_section_flags, to do this work.  This approach will also make the hooks smaller.

  Sorry, I haven't noticed that from your previous patch.

Fell free to argue.

Tristan.