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]

Re: [PATCH] Make the compiler do the math 2.


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]