This is the mail archive of the gdb-patches@sourceware.cygnus.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

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



> 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.

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

- 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.

- 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;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]