Re: [PATCH/RFC] coffread.c: delete param

[Elena Zannoni <ezannoni at redhat dot com>]

Michael Elizabeth Chastain writes:
 > Nickc writes:
 > > Given Andrew's comment in the code, I would be rather wary of this
 > > patch.  Presumably there is some good reason for passing the
 > > cs->c_sclass field in the (void *) pointer argument slot, or otherwise
 > > Andrew would not have gone to all that trouble of casting it.
 > 
 > I figured out some of the history:
 > 
 > Back in gdb 4.17, arm_pc_is_thumb in arm-tdep.c used this c_sclass
 > information to determine whether a symbol is a Thumb or Arm function.
 > gdb 4.18 rewrote arm_pc_is_thumb with a new mechanism based on
 > COFF_MAKE_MSYMBOL_SPECIAL, which sets a flag bit instead of storing the
 > sclass and reading it later.  That enables other readers besides the
 > coff reader to mark functions as special (ELF_MAKE_MSYMBOL_SPECIAL
 > in particular).
 > 
 > The change from 4.17 to 4.18 added the new COFF_MAKE_MSYMBOL_SPECIAL,
 > and changed arm-tdep.c to use the new COFF_MAKE_MSYMBOL_SPECIAL,
 > but did not delete the old dangling c_sclass code.
 > 
 > Someone with access to the old Cygnus CVS repository might be able
 > to say more about the exact change some time between 4.17 and 4.18.

Did you see my posting with the exact diffs (from 1998) I posted
yesterday?
http://sources.redhat.com/ml/gdb-patches/2003-10/msg00470.html

That explains in detail what happened, which is pretty much how you
describe it here.

elena


 > 
 > Andrew C came along a few years later and added a cast just to make
 > the compiler happy -- just an innocent janitor.
 > 
 > It's very unlikely that Elena's change will change the behavior of gdb.
 > This c_sclass info has been dead since 4.18, replaced by
 > COFF_MAKE_MSYMBOL_SPECIAL.
 > 
 > It would be nice if someone could build and test arm-coff to verify
 > this line of reasoning.  But, it's also nice to know the history,
 > which explains why c_sclass is stored, but never read.
 > 
 > > Does it improve things ?  :-)  If so, what ?
 > 
 > It's part of a cleanup for the msymbol.info field.
 > 
 > There are three different places in gdb which use msymbol.info.
 > This is one of them.  The improvement is that getting rid
 > of dead code makes it simpler to talk about the other two places.
 > 
 > Michael C