Re: [patch]: dllwrap ported for x64 and support for new --[no-]leading-underscore option

[Kai Tietz <ktietz70 at googlemail dot com>]

Hi Dave,

2009/11/4 Dave Korn <dave.korn.cygwin@googlemail.com>:
> Kai Tietz wrote:
>
>> Tested for x86_64-pc-mingw32, i686-pc-mingw32, and for i686-cygwin. Ok
>> for apply?
>
> ?Not quite. ?What is all this stuff about the "host"? ?You appear to be
> referring to the cpu type of the *target* by it, rather than any property of
> the host machine, and of course I don't suppose you actually mean to imply
> that the output should depend on properties of the system on which the
> toolchain is running, rather than the properties of the system on which the
> output executable images will be run.
Well, I changed it. What me confused here is, that normally for
configure --host is used as build target, and --target isn't necessary
the same. So as this tool is a host tool, I was confused here.

> ?The terms "host" and "target" are already well defined in the gnu build
> system and we should not muddy the waters like this. ?Please rename host_type
> and which_host to completely remove the word 'host' in all its forms; I would
> suggest you replace it by "target_cpu" or similar.
>
> ?Other than that:
>
>> + ? ? ?const char *preFix = (is_leading_underscore != 0 ? "_" : "");
>> + ? ? ?const char *postFix = "";
>> + ? ? ?const char *nameEntry;
>
> ?There is no camel-case anywhere else in this file that I could see. ?Please
> don't gratuitously introduce new and inconsistent coding style just to satisfy
> a personal preference. ?I wouldn't complain if the file was already full of
> mixed styles, but it isn't yet, so let's not start.

Agreed, just done by habit ;)

> ?The rest of it looks ok. ?There's one place the formatting looked a little
> peculiar:
>
>> + ?else if (!strncmp (target, "x86_64", 6)
>> + ? ? ? ? ? || !strncmp (target, "athlon64", 8)
>> + ? ? ? ?|| !strncmp (target, "amd64", 5))
>> + ? ?which_host = X64_HOST;
>> + ?else if (target[0] == 'i' && (target[1] >= '3' && target[1] <= '6')
Intend was ok, just one time with spaces, and the other time with tab
and spaces. I correct it, so that both lines are using the same
spacing.

> but I won't worry about formatting issues until we see your respin without the
> "host" confusion. ?Thanks for your patience.
>
> ? ?cheers,
> ? ? ?DaveK
>
>
>

Here is the updated patch

2009-11-04  Kai Tietz  <kai.tietz@onevision.com>

       * dllwrap.c (is_leading_underscore): New variable.
       (cpu_type): New enum type.
       (which_cpu): New variable.
       (usage): Add new options --no-leading-underscore
       and --leading-underscore.
       (long_options): Likewise.
       (OPTION_NO_LEADING_UNDERSCORE): New define.
       (OPTION_LEADING_UNDERSCORE): Likewise.
       (main): Initialize which_host, pass new options
       to dlltool, do underscoring dependent to
       is_leading_underscore, and do '@12' decoration
       only for x86.


Cheers,
Kai


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

Attachment: dllwrap_x64.diff
Description: Binary data