This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Fix dlls for non underscored targets.
On Thu, Sep 21, 2006 at 03:09:46AM +0100, Pedro Alves wrote:
>Christopher Faylor escreveu:
>>>>Pedro Alves wrote:
>>>>That test only runs on native configurations (*),
>>>>so I missed some obvious mistakes I made.
>>>>
>>>>(*) Can see why. It only compiles and links. It doesn't run the
>>>>built executables.
>>>>
>s/Can/Can't/
>
>
>>> for (i = 0; i < d->num_exports; i++)
>>>@@ -617,8 +619,8 @@ process_def_file (bfd *abfd ATTRIBUTE_UN
>>>
>>> /* We should not re-export imported stuff. */
>>> {
>>>- char *name = xmalloc (strlen (sn) + 2 + 6);
>>>- sprintf (name, "%s%s", U("_imp_"), sn);
>>>+ char *name = xmalloc (sizeof ("__imp_") - 1 + strlen
>>>(sn) + 1);
>>>
>>
>>Wouldn't it be better to just get rid of the "- 1 ... + 1" above?
>>
>>
>Well, I left it for clarity. The first is because sizeof ("string") returns
>the length+1 and the last is for the terminating '\0' of the full built
>'char *name'.
>It think it is clearer this way than how it was.
If you're going for clarity, then it is clearer to just use strlen
("__imp_") since that should just resolve to a constant 6.
Nevertheless, I don't agree that the +1 - 1 is clearer. Any C
programmer should know that sizeof ("a string") includes the NUL byte
and the additional arithmetic actually gave me pause for a second.
>(While doing this sizeof ("string") - 1for the nth time, I wonder if a
>#define LIT_STR_LENGTH(STR) (sizeof (STR "") - 1)
>shouldn't be added to bfd-in.h ...)
>
>>I'm also wondering if it would make sense to just put the "__imp_"/"_imp_"
>>string in a
>>variable somewhere and just use that.
>>
>>
>
>Humm, not sure what you mean? With this patch it is __imp_
>everywhere, except one place (auto_export) that expects a symbol name
>stripped from the underscore.
>Maybe you meant a define?
>
>>>
>>
>>I am not at a place where I can check the source code right now but I'm not
>>clear on why you remove the U ("_head_") in some cases and add it in
>>others.
>
>Where?
I should have just said that "remove the U()", as in:
@@ -1918,7 +1920,7 @@ make_one (def_file_export *exp, bfd *par
BSF_GLOBAL, 0);
if (! exp->flag_data)
quick_symbol (abfd, "", exp->internal_name, "", tx, BSF_GLOBAL, 0);
- quick_symbol (abfd, U ("_imp_"), exp->internal_name, "", id5,
+ quick_symbol (abfd, "__imp_", exp->internal_name, "", id5,
BSF_GLOBAL, 0);
cgf