Re: [patch]: Allow in dlltool addiional underscoring options for symbols

[Kai Tietz <ktietz70 at googlemail dot com>]

2009/10/27 Dave Korn <dave.korn.cygwin@googlemail.com>:
> 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.

Ok, I'll add some testcases for this new options. But you are right,
that we have in dlltool, ld, etc pretty much same code about import
symbols and underscoring. I thought already, if it wouldn't be good
here to handle this underscoring stuff (at least the value and symbol
name generation) directly in bfd. Also I think it could make sense to
add code for import-library entries directly in bfd, too. This would
allow us to support those new import objects and transform them into a
COFF object to handle it.

> ? ?cheers,
> ? ? ?DaveK
>
>

Cheers,
Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination