This is the mail archive of the
gdb@sourceware.org
mailing list for the GDB project.
Re: [RFC-v2] BFD MinGW/Cygwin build error in bfd/peiXXgen.c
- From: nick clifton <nickc at redhat dot com>
- To: Pierre Muller <pierre dot muller at ics-cnrs dot unistra dot fr>, "'Pedro Alves'" <palves at redhat dot com>, "'asmwarrior'" <asmwarrior at gmail dot com>
- Cc: "'GDB Development'" <gdb at sourceware dot org>, "'Binutils Development'" <binutils at sourceware dot org>
- Date: Thu, 19 Dec 2013 12:38:28 +0000
- Subject: Re: [RFC-v2] BFD MinGW/Cygwin build error in bfd/peiXXgen.c
- Authentication-results: sourceware.org; auth=none
- References: <52B17083 dot 7040404 at gmail dot com> <52B1738D dot 4010409 at redhat dot com> <002101cefbf1$7554b9a0$5ffe2ce0$ at muller@ics-cnrs.unistra.fr> <52B1E03E dot 9010002 at redhat dot com> <006c01cefcaf$bec215b0$3c464110$ at muller@ics-cnrs.unistra.fr>
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