Re: [PATCH] add-symbol-file syntax cleanup

[Elena Zannoni <ezannoni at cygnus dot com>]

Jim Blandy writes:
 > 
 > > This patch cleans up the syntax and implementation of add-symbol-file.
 > > It is the first of a series of patches that will allow to dynamically
 > > load a file and specify different sections and addresses.
 > > 
 > > Elena
 > > 
 > > 2000-04-13  Elena Zannoni  <ezannoni@kwikemart.cygnus.com>
 > > 
 > > 	* symfile.c (add_symbol_file_command): Rewrite the arguments
 > >  	processing part. Simplify syntax of command.
 > > 	(_initialize_symfile): Update help message for add-symbol-file
 > >  	command.
 > 
 > Approved, with some comments:
 > 
 > - What happens if the command line has trailing spaces?  If I follow
 >   the logic, I think you'll get an error.
 > 

Probably, I didn't think about that. I'll look it over and fix it if
needed.

 > - You register a lot of cleanups, but you never call do_cleanups or
 >   discard_cleanups.

Yes, right. Thanks for the reminder. Some of the cleanups were in
there already, and do_cleanups wasn't called. I hadn't noticed. I'll
fix it.

 > 
 > - Do you actually need to dup those strings at all?  It seems to me
 >   that the arg list itself will be live at least until the function
 >   returns.  If your cleanups for the xstrdup calls are meant to be run
 >   when the function returns, you might as well not dup them at all.
 > 

True.

 > - Please format structures like this:
 > 
 >       if (strcmp (sec, ".text") == 0)
 >         section_addrs.text_addr = addr;
 >       else 
 >         if (strcmp (sec, ".data") == 0)
 >           section_addrs.data_addr = addr;
 >         else 
 >           if (strcmp (sec, ".bss") == 0)
 >             section_addrs.bss_addr = addr;
 > 
 >   this way:
 > 
 >       if (strcmp (sec, ".text") == 0)
 >         section_addrs.text_addr = addr;
 >       else if (strcmp (sec, ".data") == 0)
 >         section_addrs.data_addr = addr;
 >       else if (strcmp (sec, ".bss") == 0)
 >         section_addrs.bss_addr = addr;


Ah, Ok, They were the way you wanted it before. I didn't know about this. 

Thanks
Elena