Re: [RFC-v2] BFD MinGW/Cygwin build error in bfd/peiXXgen.c

[nick clifton <nickc@redhat.com>]

Hi Pierre,

>    2) In GDB coding rules, we are not allowed to mix code and declaration.
> The declaration of res after handling the ! is_name part
> does not follow this rule.
>    Isn't that rule also used for Binutils project?

Doh - this is my bad.  The rule *is* followed in the Binutils project 
and I should have caught this before checking in the patch.

I blame the GCC project - ever since they switched over to using C++ I 
have been developing "bad" C coding habits.


>    I moved the declaration of astring, alen, bstring and blen to
> the start of the function. It does introduce a penalty, in the sense that
> those variables are set even though this is not useful if is_name is false.

I do not see this as a problem.  I am much more interested in clean code 
than in highly optimized, but harder to read code.


> Another silly coding style question: Should there by a space
> between the address operator "&" and the variable or expression following?

*sigh*  There is no strict rule on this subject.  Most people omit the 
space, I include it.  I feel that since we separate other operators from 
their arguments we should do the same with &.  I feel the same way about 
the asterisk operator too, but not many people agree with me on that one 
either.


>    3) I was wondering why the Windows case started at astring + 2,
> but I couldn't find any valid reason, so that I changed it to
> compare the strings from the first position.

Doh - I was confused by the length field which I thought was included in 
my string pointer.


>    Finally, I also have a question regarding the codepage field.
> According to Windows PE resources can use any codepage.
> Is the new code restricted to UNICODE only?
> If not, shouldn't the rsrc_cmp code also depend on whether
> 16-bit Windows UNICODE is used or any other page code?

I have no idea. :-(  I am not an expert on this subject.  According to 
the "Microsoft Portable Executable and Common Object File Format 
Specification Revision 8.3 – February 6, 2013" which has been my guide 
for this work, the codepage field is:

    The code page that is used to decode code point values
    within the resource data. Typically, the code page would be
    the Unicode code page.

But Resource Directory Strings are:

   Offset  Size    Field    Description
   0       2       Length   The size of the string, not including length
                            field itself.

   2      variable Unicode  The variable-length Unicode string data,
                   String   word-aligned.

Ie - the strings are explicitly encoded in Unicode.  I think that the 
codepage field in a resource entry only applies to non-resource-string data.


> 2013-12-19  Pierre Muller  <muller@sourceware.org>
>
> 	peXXigen.c (u16_mbtouc): Avoid unused function warning by excluding if
> 	__CYGWIN__ or __MINGW32__ macro is defined.
> 	(rsrc_cmp): Fix Windows host version and version without wchar header.
> 	[__CYGWIN__, __MINGW32__]: Introduce rsrccmp macro.

Thanks - I have applied this patch.

Cheers
   Nick