Re: PATCH: some more tidy-ups

Re: PATCH: some more tidy-ups

[Ben Elliston]

Hi Alan,

(Sorry for the broken threading; I dig up the quotes for this message
from the mailing list archives, so the References: will be wrong).

> > A couple of more tidy-ups.  My change to linker.c should now prevent
> > the possibility of a segfault in BFD due to a null pointer dereference
> > (well, at least, it at least pushes it up a level!)

> I don't see any dereference.

No, but let's pull up the original source:

   886    /* Call the allocation method of the superclass.  */
   887    ret = ((struct archive_hash_entry *)
   888           bfd_hash_newfunc ((struct bfd_hash_entry *) ret, table, string));
   889  
   890    if (ret)
   891      {
   892        /* Initialize the local fields.  */
   893        ret->defs = NULL;
   894      }
   895  
   896    return &ret->root;
   897  }

If the call to bfd_hash_newfunc returns NULL, then we skip
initialisation, but return &ret->root.  It's lucky that `root' happens
to be the first member of the structure, so that the result of the
address expression is still 0.  If the composition of this structure
were to ever change, we might return 4 or 8 in the failure case.

At the very least, this needs a loud comment.  :-)
Do you agree?

Ben

Re: PATCH: some more tidy-ups

[Alan Modra]

On Tue, May 09, 2006 at 04:10:10PM +1000, Ben Elliston wrote:
> If the call to bfd_hash_newfunc returns NULL, then we skip
> initialisation, but return &ret->root.  It's lucky that `root' happens
> to be the first member of the structure, so that the result of the
> address expression is still 0.  If the composition of this structure

It's not luck, it's by design.  :^)  I hardly think it needs a comment,
but feel free to add something, preferably a single line.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: some more tidy-ups

[Alan Modra]

On Tue, May 09, 2006 at 11:39:27AM +1000, Ben Elliston wrote:
> A couple of more tidy-ups.  My change to linker.c should now prevent
> the possibility of a segfault in BFD due to a null pointer dereference
> (well, at least, it at least pushes it up a level!)

I don't see any dereference.

> Tested with an all-targets build.  Okay for mainline?
> 
> 
> 2006-05-09  Ben Elliston  <bje@au.ibm.com>
> 
> 	* linker.c (archive_hash_newfunc): Guard against a null return
> 	value from bfd_hash_newfunc.
> 	* elf64-ppc.c (ppc64_elf_finish_dynamic_symbol): Remove unused
> 	local variable `dynobj'.

The elf64-ppc.c change is OK, thanks!

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

PATCH: some more tidy-ups

[Ben Elliston]

A couple of more tidy-ups.  My change to linker.c should now prevent
the possibility of a segfault in BFD due to a null pointer dereference
(well, at least, it at least pushes it up a level!)

Tested with an all-targets build.  Okay for mainline?


2006-05-09  Ben Elliston  <bje@au.ibm.com>

	* linker.c (archive_hash_newfunc): Guard against a null return
	value from bfd_hash_newfunc.
	* elf64-ppc.c (ppc64_elf_finish_dynamic_symbol): Remove unused
	local variable `dynobj'.

Index: linker.c
===================================================================
RCS file: /cvs/src/src/bfd/linker.c,v
retrieving revision 1.53
diff -u -p -r1.53 linker.c
--- linker.c	16 Mar 2006 12:20:16 -0000	1.53
+++ linker.c	9 May 2006 01:34:46 -0000
@@ -886,13 +886,11 @@ archive_hash_newfunc (struct bfd_hash_en
   /* Call the allocation method of the superclass.  */
   ret = ((struct archive_hash_entry *)
 	 bfd_hash_newfunc ((struct bfd_hash_entry *) ret, table, string));
+  if (ret == NULL)
+    return NULL;
 
-  if (ret)
-    {
-      /* Initialize the local fields.  */
-      ret->defs = NULL;
-    }
-
+  /* Initialize the local fields.  */
+  ret->defs = NULL;
   return &ret->root;
 }
Index: elf64-ppc.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-ppc.c,v
retrieving revision 1.237
diff -u -p -r1.237 elf64-ppc.c
--- elf64-ppc.c	5 May 2006 13:07:30 -0000	1.237
+++ elf64-ppc.c	9 May 2006 01:35:14 -0000
@@ -11102,13 +11102,11 @@ ppc64_elf_finish_dynamic_symbol (bfd *ou
 				 Elf_Internal_Sym *sym)
 {
   struct ppc_link_hash_table *htab;
-  bfd *dynobj;
   struct plt_entry *ent;
   Elf_Internal_Rela rela;
   bfd_byte *loc;
 
   htab = ppc_hash_table (info);
-  dynobj = htab->elf.dynobj;
 
   for (ent = h->plt.plist; ent != NULL; ent = ent->next)
     if (ent->plt.offset != (bfd_vma) -1)