Re: [patch v9 1/5] New remove-symbol-file command.

[Luis Machado <lgustavo at codesourcery dot com>]

Found a few more nits.

> @@ -7457,6 +7457,66 @@ disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
>    }
>  }
>
> +/* Disable any breakpoints and tracepoints in OBJFILE upon
> +   notification of free_objfile.  Only apply to enabled breakpoints,
> +   disabled ones can just stay disabled.  */
> +
> +static void
> +disable_breakpoints_in_freed_objfile (struct objfile * objfile)

struct objfile *objfile

> +{
> +  struct breakpoint *b;
> +
> +  if (objfile == NULL)
> +    return;
> +
> +  /* If the file is a shared library not loaded by the user then
> +     solib_unloaded was notified and disable_breakpoints_in_unloaded_shlib
> +     was called.  In that case there is no need to take action again.  */
> +  if ((objfile->flags & OBJF_SHARED) && !(objfile->flags & OBJF_USERLOADED))
> +    return;
> +
> +  ALL_BREAKPOINTS (b)
> +  {

Identation is odd here. Should be further in.

> @@ -1463,6 +1466,26 @@ resume_section_map_updates_cleanup (void *arg)
>    resume_section_map_updates (arg);
>  }
>
> +/* Return 1 if ADDR maps into one of the sections of OBJFILE and 0
> +   otherwise.  */
> +
> +int
> +is_addr_in_objfile (CORE_ADDR addr, const struct objfile * objfile)

const struct objfile *objfile

> +{
> +  struct obj_section *osect;
> +
> +  if (objfile == NULL)
> +    return 0;
> +
> +  ALL_OBJFILE_OSECTIONS (objfile, osect)
> +  {

Identation off as well. Should be further in.

> diff --git a/gdb/objfiles.h b/gdb/objfiles.h
> index adb1ef8..b25afb1 100644
> --- a/gdb/objfiles.h
> +++ b/gdb/objfiles.h
> @@ -482,6 +482,8 @@ extern int have_full_symbols (void);
>
>  extern void objfiles_changed (void);
>
> +extern int is_addr_in_objfile (CORE_ADDR, const struct objfile *);
> +

Prototype arguments should be named as in the rest of the prototypes of
this header file.

> diff --git a/gdb/solib.c b/gdb/solib.c
> index c987fe5..7ceef29 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -1478,6 +1478,26 @@ gdb_bfd_lookup_symbol (bfd *abfd,
>    return symaddr;
>  }
>
> +/* SO_LIST_HEAD may contain user-loaded object files that can be removed
> +   out-of-band by the user.  So upon notification of free_objfile remove
> +   any reference to any user-loaded file that is about to be freed.  */
> +


while at it, "any references to any user-loaded...".

> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index c2ad797..f7ad268 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -2330,6 +2330,81 @@ add_symbol_file_command (char *args, int from_tty)
>  }
>  ^L
>
> +/* This function removes a symbol file that was added via add-symbol-file.  */
> +
> +static void
> +remove_symbol_file_command (char *args, int from_tty)
> +{
> +  char **argv;
> +  struct objfile* objf = NULL;
> +  struct cleanup *my_cleanups;
> +  struct program_space *pspace = current_program_space;
> +  struct gdbarch *gdbarch = get_current_arch ();
> +
> +  dont_repeat ();
> +
> +  if (args == NULL)
> +    error (_("USAGE: remove-symbol-file FILENAME\n\
> +       remove-symbol-file -a ADDRESS"));

I discovered something odd. The only two commands using "USAGE" in help
texts are this one and add-symbol-file. I think we should drop it.

Moreover, add-symbol-file prints this when ran without arguments:

(gdb) add-symbol-file
add-symbol-file takes a file name and an address


And this when ran with more arguments than it should:

USAGE: add-symbol-file <filename> <textaddress> [-readnow]
[-s <secname> <addr>]*

So it is a little odd. We should probably fix this in another patch. For 
this
one i think we should only drop USAGE.

> +
> +  my_cleanups = make_cleanup (null_cleanup, NULL);
> +
> +  argv = gdb_buildargv (args);
> +
> +  if (strcmp (argv[0], "-a") == 0)
> +    {
> +      /* Interpret the next argument as an address.  */
> +      CORE_ADDR addr;
> +
> +      if (argv[1] == NULL)
> +       error (_("Missing address argument"));
> +
> +      if (argv[2] != NULL)
> +       error (_("Junk after %s"), argv[1]);
> +
> +      addr = parse_and_eval_address (argv[1]);
> +
> +      ALL_OBJFILES (objf)
> +      {

Identation off here. Should be further in.

> +       if (objf->flags & OBJF_USERLOADED
> +           && objf->pspace == pspace
> +           && is_addr_in_objfile (addr, objf))
> +         break;
> +      }
> +    }
> +  else if (argv[0] != NULL)
> +    {
> +      /* Interpret the current argument as a file name.  */
> +      char *filename;
> +
> +      if (argv[1] != NULL)
> +       error (_("Junk after %s"), argv[0]);
> +
> +      filename = tilde_expand (argv[0]);
> +      make_cleanup (xfree, filename);
> +
> +      ALL_OBJFILES (objf)
> +      {

Identation off as well. Should be further in.