public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* rel/rela patch for get_dynamic_reloc_section_name
@ 2011-02-16  5:29 Bernd Schmidt
  2011-02-22 15:31 ` Nick Clifton
  0 siblings, 1 reply; 6+ messages in thread
From: Bernd Schmidt @ 2011-02-16  5:29 UTC (permalink / raw)
  To: binutils

[-- Attachment #1: Type: text/plain, Size: 481 bytes --]

In get_dynamic_reloc_section_name, we try to find the name of an
existing reloc section, then we call is_reloc_section to make sure it's
the name we expected. This fails when there are both rel and rela input
sections, as was seen when trying to link C6X objects that were compiled
with TI's compiler using GNU ld.

Instead, this patch makes us just build the string we want and return
it. Not only is this simpler, it also appears to fix the problem
mentioned above.

Ok?


Bernd

[-- Attachment #2: dynrelname.diff --]
[-- Type: text/plain, Size: 1867 bytes --]

	bfd/
	* elflink.c (is_reloc_section): Remove function.
	(get_dynamic_reloc_section_name): Construct string manually.

Index: bfd/elflink.c
===================================================================
--- bfd/elflink.c	(revision 307279)
+++ bfd/elflink.c	(working copy)
@@ -12542,20 +12542,6 @@ _bfd_elf_default_got_elt_size (bfd *abfd
 
 /* Routines to support the creation of dynamic relocs.  */
 
-/* Return true if NAME is a name of a relocation
-   section associated with section S.  */
-
-static bfd_boolean
-is_reloc_section (bfd_boolean rela, const char * name, asection * s)
-{
-  if (rela)
-    return CONST_STRNEQ (name, ".rela")
-      && strcmp (bfd_get_section_name (NULL, s), name + 5) == 0;
-
-  return CONST_STRNEQ (name, ".rel")
-    && strcmp (bfd_get_section_name (NULL, s), name + 4) == 0;
-}
-
 /* Returns the name of the dynamic reloc section associated with SEC.  */
 
 static const char *
@@ -12563,26 +12549,19 @@ get_dynamic_reloc_section_name (bfd *   
 				asection *  sec,
 				bfd_boolean is_rela)
 {
-  const char * name;
-  unsigned int strndx = elf_elfheader (abfd)->e_shstrndx;
-  unsigned int shnam = _bfd_elf_single_rel_hdr (sec)->sh_name;
+  char *name;
+  const char *old_name = bfd_get_section_name (NULL, sec);
 
-  name = bfd_elf_string_from_elf_section (abfd, strndx, shnam);
-  if (name == NULL)
+  if (old_name == NULL)
     return NULL;
 
-  if (! is_reloc_section (is_rela, name, sec))
-    {
-      static bfd_boolean complained = FALSE;
+  name = bfd_alloc (abfd, (is_rela ? 6 : 5) + strlen (old_name));
 
-      if (! complained)
-	{
-	  (*_bfd_error_handler)
-	    (_("%B: bad relocation section name `%s\'"),  abfd, name);
-	  complained = TRUE;
-	}
-      name = NULL;
-    }
+  if (is_rela)
+    strcpy (name, ".rela");
+  else
+    strcpy (name, ".rel");
+  strcat (name, old_name);
 
   return name;
 }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: rel/rela patch for get_dynamic_reloc_section_name
  2011-02-16  5:29 rel/rela patch for get_dynamic_reloc_section_name Bernd Schmidt
@ 2011-02-22 15:31 ` Nick Clifton
  2011-02-22 15:35   ` H.J. Lu
  2011-02-22 15:42   ` Bernd Schmidt
  0 siblings, 2 replies; 6+ messages in thread
From: Nick Clifton @ 2011-02-22 15:31 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: binutils

Hi Bernd,

+  name = bfd_alloc (abfd, (is_rela ? 6 : 5) + strlen (old_name));
+  if (is_rela)
+    strcpy (name, ".rela");
+  else
+    strcpy (name, ".rel");
+  strcat (name, old_name);

I dislike seeing magic constants like "6" and "5" in the code above. 
How about rewriting this as:

   const char * prefix = is_rela ? ".rela" : ".rel";
   name = bfd_alloc (abfd, strlen (prefix) + strlen (old_name) + 1);
   sprintf (name, "%s%s", prefix, old_name);

Cheers
   Nick

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: rel/rela patch for get_dynamic_reloc_section_name
  2011-02-22 15:31 ` Nick Clifton
@ 2011-02-22 15:35   ` H.J. Lu
  2011-02-22 15:39     ` Nick Clifton
  2011-02-22 15:42   ` Bernd Schmidt
  1 sibling, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2011-02-22 15:35 UTC (permalink / raw)
  To: Nick Clifton; +Cc: Bernd Schmidt, binutils

On Tue, Feb 22, 2011 at 7:31 AM, Nick Clifton <nickc@redhat.com> wrote:
> Hi Bernd,
>
> +  name = bfd_alloc (abfd, (is_rela ? 6 : 5) + strlen (old_name));
> +  if (is_rela)
> +    strcpy (name, ".rela");
> +  else
> +    strcpy (name, ".rel");
> +  strcat (name, old_name);
>
> I dislike seeing magic constants like "6" and "5" in the code above. How
> about rewriting this as:
>
>  const char * prefix = is_rela ? ".rela" : ".rel";
>  name = bfd_alloc (abfd, strlen (prefix) + strlen (old_name) + 1);
>  sprintf (name, "%s%s", prefix, old_name);
>

Why not just use (is_rela ? sizeof (.rela) -1: sizeof (.rel) - 1)



-- 
H.J.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: rel/rela patch for get_dynamic_reloc_section_name
  2011-02-22 15:35   ` H.J. Lu
@ 2011-02-22 15:39     ` Nick Clifton
  0 siblings, 0 replies; 6+ messages in thread
From: Nick Clifton @ 2011-02-22 15:39 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Bernd Schmidt, binutils

Hi H.J.

>> +  name = bfd_alloc (abfd, (is_rela ? 6 : 5) + strlen (old_name));
>> +  if (is_rela)
>> +    strcpy (name, ".rela");
>> +  else
>> +    strcpy (name, ".rel");
>> +  strcat (name, old_name);

>>   const char * prefix = is_rela ? ".rela" : ".rel";
>>   name = bfd_alloc (abfd, strlen (prefix) + strlen (old_name) + 1);
>>   sprintf (name, "%s%s", prefix, old_name);

> Why not just use (is_rela ? sizeof (.rela) -1: sizeof (.rel) - 1)

Because with that version you still have to type in the prefix strings 
twice, once for the sizeof computation and once for the string creation. 
  With my version the strings are only typed in once giving less chance 
for a typo and a clearer indication that the size of the name buffer is 
related to the strings that are being placed into it.

Cheers
   Nick


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: rel/rela patch for get_dynamic_reloc_section_name
  2011-02-22 15:31 ` Nick Clifton
  2011-02-22 15:35   ` H.J. Lu
@ 2011-02-22 15:42   ` Bernd Schmidt
  2011-02-22 17:28     ` Nick Clifton
  1 sibling, 1 reply; 6+ messages in thread
From: Bernd Schmidt @ 2011-02-22 15:42 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

On 02/22/2011 04:31 PM, Nick Clifton wrote:
> Hi Bernd,
> 
> +  name = bfd_alloc (abfd, (is_rela ? 6 : 5) + strlen (old_name));
> +  if (is_rela)
> +    strcpy (name, ".rela");
> +  else
> +    strcpy (name, ".rel");
> +  strcat (name, old_name);
> 
> I dislike seeing magic constants like "6" and "5" in the code above. How
> about rewriting this as:
> 
>   const char * prefix = is_rela ? ".rela" : ".rel";
>   name = bfd_alloc (abfd, strlen (prefix) + strlen (old_name) + 1);
>   sprintf (name, "%s%s", prefix, old_name);

Ok with that change?


Bernd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: rel/rela patch for get_dynamic_reloc_section_name
  2011-02-22 15:42   ` Bernd Schmidt
@ 2011-02-22 17:28     ` Nick Clifton
  0 siblings, 0 replies; 6+ messages in thread
From: Nick Clifton @ 2011-02-22 17:28 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: binutils

Hi Bernd,

>> +  name = bfd_alloc (abfd, (is_rela ? 6 : 5) + strlen (old_name));
>> +  if (is_rela)
>> +    strcpy (name, ".rela");
>> +  else
>> +    strcpy (name, ".rel");
>> +  strcat (name, old_name);
>>
>> I dislike seeing magic constants like "6" and "5" in the code above. How
>> about rewriting this as:
>>
>>    const char * prefix = is_rela ? ".rela" : ".rel";
>>    name = bfd_alloc (abfd, strlen (prefix) + strlen (old_name) + 1);
>>    sprintf (name, "%s%s", prefix, old_name);
>
> Ok with that change?

Yup. :-)

Cheers
   Nick


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-02-22 17:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-16  5:29 rel/rela patch for get_dynamic_reloc_section_name Bernd Schmidt
2011-02-22 15:31 ` Nick Clifton
2011-02-22 15:35   ` H.J. Lu
2011-02-22 15:39     ` Nick Clifton
2011-02-22 15:42   ` Bernd Schmidt
2011-02-22 17:28     ` Nick Clifton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).