Re: [PATCH] Fix dlls for non underscored targets.

[Christopher Faylor <cgf-use-the-mailinglist-please at sourceware dot org>]

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