[PATCH] Make the compiler do the math 2.

[Pedro Alves]

Nick Clifton wrote:
> Hi Andreas, Hans-Peter, Pedro,
> 
>> --- bfd/elflink.c.~1.230.~    2006-09-18 10:58:08.000000000 +0200
>> +++ bfd/elflink.c    2006-09-25 22:34:35.000000000 +0200
>> @@ -9699,13 +9699,16 @@ bfd_elf_gc_sections (bfd *abfd, struct b
>>          unsigned long len;
>>          char *fn_name;
>>          asection *fn_text;
>> +        int o_name_prefix_len = strlen (".gcc_except_table.");
>> +        int fn_name_prefix_len = strlen (".text.");
>>  
>> -        len = strlen (o->name + 18) + 1;
>> -        fn_name = bfd_malloc (len + 6);
>> +        len = strlen (o->name + o_name_prefix_len) + 1;
>> +        fn_name = bfd_malloc (len + fn_name_prefix_len);
>>          if (fn_name == NULL)
>>            return FALSE;
>> -        memcpy (fn_name, STRING_COMMA_LEN (".text."));
>> -        memcpy (fn_name + 6, o->name + 18, len);
>> +        strcpy (fn_name, ".text.");
>> +        memcpy (fn_name + fn_name_prefix_len,
>> +            o->name + o_name_prefix_len, len);
>>          fn_text = bfd_get_section_by_name (sub, fn_name);
>>          free (fn_name);
>>          if (fn_text == NULL || !fn_text->gc_mark)
> 
> 
> You know, I was looking at this code, and tweaking it with Pedro's 
> version of the patch when it occurred to me that using memcpy or strcpy 
> here is not the best thing to do.  It obscures what is going on, and 
> whilst it may be more efficient, it makes the code harder to read. 
> Personally I would prefer to use the slower sprintf() function as I 
> think that makes things clearer.  Like this:
> 
>   if (CONST_STRNEQ (o->name, GCC_EXCEPT_TABLE))
>     {
>       char *fn_name;
>       char *sec_name;
>       asection *fn_text;
>       unsigned o_name_prefix_len  = sizeof (GCC_EXCEPT_TABLE) - 1;
>       unsigned fn_name_prefix_len = sizeof (DOT_TEXT_DOT) - 1;
> 
>       sec_name = o->name + o_name_prefix_len;
>       fn_name = bfd_malloc (strlen (sec_name) + fn_name_prefix_len + 1);
>       if (fn_name == NULL)
>         return FALSE;
>       sprintf (fn_name, DOT_TEXT_DOT "%s", sec_name);
>       fn_text = bfd_get_section_by_name (sub, fn_name);
>       free (fn_name);
>       if (fn_text == NULL || !fn_text->gc_mark)
>         continue;
>     }
> 
>   With suitable #define's for DOT_TEXT_DOT and GCC_EXCEPT_TABLE of 
> course.  What do you think ?
> 
>   Actually I think that it might be clearer if we used two %s operators 
> inside the sprintf like this:
> 
>       sprintf (fn_name, "%s%s", DOT_TEXT_DOT, sec_name);
> 

Agreed.

Cheers,
Pedro Alves

> Cheers
>   Nick