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

[Dave Korn <dave dot korn dot cygwin at googlemail dot 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.

  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.

  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')

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

    cheers,
      DaveK