[patch PE-COFF]: Some minor nits

Kai Tietz ktietz70@googlemail.com
Wed Mar 18 18:54:00 GMT 2009


2009/3/18 Dave Korn <dave.korn.cygwin@googlemail.com>:
> Kai Tietz wrote:
>
>> Thanks Dave for reviewing it. Here the adjusted patch as discussed.
>
>  Oh, sorry to be such a nuisance, I've just spotted another potential hiccup,
> only minor though:
>
> @@ -1245,6 +1244,8 @@
>     sec_flags |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
>  #endif
>
> +  section->flags = sec_flags;
> +
>   if (flags_ptr)
>     * flags_ptr = sec_flags;
>
>
>  This changes the behaviour of the function styp_to_sec_flags.  As far as I
> could see the only client is this code in make_a_section_from_file in coffgen.c:
>
>
>  return_section->lineno_count = hdr->s_nlnno;
>  return_section->userdata = NULL;
>  return_section->next = NULL;
>  return_section->target_index = target_index;
>
>  if (! bfd_coff_styp_to_sec_flags_hook (abfd, hdr, name, return_section,
>                                         & flags))
>    result = FALSE;
>
>  return_section->flags = flags;
>
> ... so it will make no difference in this case, but I still think it isn't
> right for what should be a simple conversion function returning an integer to
> modify one of its input parameters.  Nothing about the function name suggests
> it will modify the section object, nor the documentation comment at the top.
> I think the assignment can be safely dropped.
>
>    cheers,
>      DaveK
>

Well, you're right. The issue here was for me, that locally I removed
in the function make_a_section_from_file the assignment of
section->flags. I removed this hunk from the patch.

Cheers,
Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination
-------------- next part --------------
A non-text attachment was scrubbed...
Name: noread.diff
Type: application/octet-stream
Size: 3378 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20090318/898f1db5/attachment.obj>


More information about the Binutils mailing list