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] |
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
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |