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

nick clifton
2013-12-19T12:41:31Z

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