This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]