This is the mail archive of the binutils@sources.redhat.com 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: Prefixing sybmols/sections with objcopy


Hi Nick,

> I would like to submit the following patch to objcopy for review. It
> adds the ability to globally prefix all symbols and/or sections with
> a given prefix to shift the name space.

Thank you for submitting this patch, and my apologies for the long
delay in responding to you.
 
> A general copyright assignment has been signed by Ubicom.

Really ?  I do not have details of this in my database.  Please could
you send me a copy of the assignment.


I have some comments on the patch itself:

> + /* Prefix symbols/sections */

Formatting.  Please treat comments as sentences and terminate them
with a full stop followed by two spaces.

> +       if (prefix_symbols_string)
> +         {
> +           char *n;
> + 	  n = xmalloc (strlen (prefix_symbols_string) + strlen (name) + 1);
> +           strcpy (n, prefix_symbols_string);
> +           strcat (n, name);
> + 	  name = bfd_asymbol_name (sym) = n;
> +         }

It would be worth considering if this code could be integrated with
the code to handle 'change_leading_char' and 'redefine_sym_list', so
that only one xmalloc() is necessary to rename a symbol.  A similar
issue applied to the section renaming code.


> +   /* Prefix sections */
> +   if (prefix_sections_string)
> +     {
> +       char *n;
> +       n = xmalloc (strlen (prefix_sections_string) + strlen (name) + 1);
> +       strcpy (n, prefix_sections_string);
> +       strcat (n, name);
> +       name = n;
> +     }
> +   else if ((prefix_alloc_sections_string) && (bfd_get_section_flags(ibfd, isection) & SEC_ALLOC))

Formatting - please insert a space between a function's name and the
opening parenthesis of its argument list.

> +     {
> +       char *n;
> +       n = xmalloc (strlen (prefix_alloc_sections_string) + strlen (name) + 1);
> +       strcpy (n, prefix_alloc_sections_string);
> +       strcat (n, name);
> +       name = n;
> +     }

Unnecessary code duplication.  The body of the two if statements are
almost exactly the same.

In general however the code looks fine, and if we can clear up the
copyright assignment issue, I will be happy to accept the patch.

Cheers
        Nick


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