This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [patch]: Allow in dlltool addiional underscoring options for symbols
Kai Tietz wrote:
>> 2009-10-26 Kai Tietz <kai.tietz@
>>
>> * dlltoo.c (leading_underscore): New local variable.
Typo in "dlltool".
>> (asm_prefix): Interprete leading_underscore.
"Interpret" has no trailing 'e'. Perhaps just "Use" if you'd prefer?
Only one tiny point in the patch:
> --- src.orig/binutils/dlltool.c 2009-10-23 16:47:36.000000000 +0200
> +++ src/binutils/dlltool.c 2009-10-26 15:49:15.296692100 +0100
> @@ -394,6 +394,7 @@
> static int add_indirect = 0;
> static int add_underscore = 0;
> static int add_stdcall_underscore = 0;
> +static int leading_underscore = -1;
> static int dontdeltemps = 0;
Since this is a tri-state rather than a boolean like the rest, please add a
comment documenting the values it takes and how it interacts with
add_underscore and add_stdcall_underscore. We appear to be gradually
accumulating flags with a similar or overlapping semantics here, so let's try
and keep it simple to understand for whoever comes after us.
>> Tested for x86_64-pc-mingw32, i686-pc-mingw32, and i686-pc-cygwin. Ok for
>> apply?
Because this code is getting hairy and has all these multiple interacting
flags, I'm going to insist on testcases, but the patch is good. OK once
you've added testcases. They should verify the option works in both positive
and negative forms. For bonus points, you could make them verify the
interactions between this option and the other underscore-related options DTRT
:) but I won't insist on that part of it.
cheers,
DaveK