Re: [rfa] clean up lookup_symbol_aux

[Elena Zannoni <ezannoni at redhat dot com>]

David Carlton writes:
 > On Sun, 3 Nov 2002 19:01:23 -0500, Elena Zannoni <ezannoni@redhat.com> said:
 > > David Carlton writes:
 > >> On 02 Oct 2002 14:50:34 -0700, David Carlton
 > >> <carlton@math.Stanford.EDU> said:
 > 
 > >> > * The part of lookup_symbol_aux that never gets run and that is
 > >> >   prefaced by 17 lines of comments being bemused at its existence
 > >> >   has been deleted.
 > 
 > > Could it be if deffed 0 for a bit, to see if we get some weird
 > > behaviors?  We can delete it later. Make sure you add a comment with
 > > a date.
 > 
 > Sure, I can do that.  Having said that, I don't really think this is
 > necessary: it's pretty clear that the code isn't being run; it's
 > prefaced by multiple comments that point out that it isn't being run,
 > that can't figure out what the code is supposed to do, and that are, I
 > think, a few years old (the more recent one seems to date from the HP
 > merge).  So ifdeffing it out won't change the behavior of GDB at all,
 > it'll just preserve another one of GDB's mysteries for that much
 > longer.  But ifdeffing it won't hurt anything, either, so I can
 > certainly do it if you'd be happier.
 > 

Yes, please. It wouldn't be the first time that weird things happen
because of apparently insignificant changes.  (see the old proverbial
wait_for_inferior).

 > >> > * The two cases for searching minimal symbols (i.e. whether or not
 > >> >   you're in HPUXHPPA) turned out to be slightly different.  I went
 > >> >   through those differences and picked whichever variant seemed to
 > >> >   me to be better; as far as I can tell, there's no reason for the
 > >> >   code to differ in the two cases.  I also threw in a bug fix:
 > >> >   fixup_symbol_section had the wrong second argument in the variant
 > >> >   where it was present.
 > 
 > > This one I am a bit worried about.  For the bugfix, I would prefer
 > > that a separate patch is submitted, I cannot spot the fix in the
 > > patch.
 > 
 > Yeah, I probably should have done that first.  I'll include a patch
 > just for that after my signature; the point of the patch is that the
 > minsym code refers to 'block' and 'objfile', but those variables don't
 > actually point at anything useful in the minsym code.  (They're
 > pointing at whatever they happened to point at at the end of the
 > symtab search.)  So the patch replaces 'objfile' by something correct,

Right. s->objfile will have 's' always non-null.

 > and replaces 'block' by NULL.  (There are other possible bugfixes, but
 > they would all result from situations where the HP and non-HP
 > currently do different things: those can wait.)
 > 

Ah yes, these two would be in the 'else' which is taken if s == NULL
and block is not going to be assigned to.

 > > I would check in the functions except the minsyms one, ifdeffing the
 > > code that was deleted. Separately submit a patch for the bugfix, and
 > > one just for the minsyms changes so that it can be tested
 > > independentely.
 > 
 > Great, I'll do that.  If you can look at the bugfix one first, I'd
 > appreciate it, because I don't want to move out the
 > local/symtab/psymtab stuff before having that bugfix patch accepted.
 > (If the bugfix patch isn't accepted, moving out the other parts of the
 > code might lead to bizarre consequences.)
 > 

Ok approved. Good catch.

Elena

 > David Carlton
 > carlton@math.stanford.edu
 > 
 > 2002-11-04  David Carlton  <carlton@math.stanford.edu>
 > 
 > 	* symtab.c (lookup_symbol_aux): In minsym sections, don't use the
 > 	previous values of 'objfile' and 'block'.
 > 
 > Index: symtab.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.c,v
 > retrieving revision 1.75
 > diff -u -p -r1.75 symtab.c
 > --- symtab.c	28 Oct 2002 17:05:56 -0000	1.75
 > +++ symtab.c	4 Nov 2002 23:59:05 -0000
 > @@ -929,7 +929,7 @@ lookup_symbol_aux (const char *name, con
 >  
 >  	      if (symtab != NULL)
 >  		*symtab = s;
 > -	      return fixup_symbol_section (sym, objfile);
 > +	      return fixup_symbol_section (sym, s->objfile);
 >  	    }
 >  	  else if (MSYMBOL_TYPE (msymbol) != mst_text
 >  		   && MSYMBOL_TYPE (msymbol) != mst_file_text
 > @@ -937,7 +937,7 @@ lookup_symbol_aux (const char *name, con
 >  	    {
 >  	      /* This is a mangled variable, look it up by its
 >  	         mangled name.  */
 > -	      return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name, block,
 > +	      return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name, NULL,
 >  					namespace, is_a_field_of_this, symtab);
 >  	    }
 >  	  /* There are no debug symbols for this file, or we are looking
 > @@ -1120,7 +1120,7 @@ lookup_symbol_aux (const char *name, con
 >  		   && !STREQ (name, SYMBOL_NAME (msymbol)))
 >  	    {
 >  	      return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name,
 > -					block, namespace, is_a_field_of_this,
 > +					NULL, namespace, is_a_field_of_this,
 >  					symtab);
 >  	    }
 >  	}