From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21159 invoked by alias); 18 Jun 2013 17:26:18 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 21150 invoked by uid 89); 18 Jun 2013 17:26:17 -0000 X-Spam-SWARE-Status: No, score=-4.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_BJ,TW_RG autolearn=ham version=3.3.1 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 18 Jun 2013 17:26:13 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1UozfW-0003CH-MG from Luis_Gustavo@mentor.com ; Tue, 18 Jun 2013 10:26:10 -0700 Received: from NA1-MAIL.mgc.mentorg.com ([147.34.98.181]) by svr-orw-fem-01.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Tue, 18 Jun 2013 10:26:09 -0700 Received: from [172.30.14.165] ([172.30.14.165]) by NA1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 18 Jun 2013 10:26:09 -0700 Message-ID: <51C0982C.7080203@codesourcery.com> Date: Tue, 18 Jun 2013 17:26:00 -0000 From: Luis Machado Reply-To: lgustavo@codesourcery.com User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: Nicolas Blanc CC: gdb-patches@sourceware.org, Hafiz_Abid@mentor.com, palves@redhat.com, tromey@redhat.com, eliz@gnu.org, yao@codesourcery.com, dje@google.com Subject: Re: [patch v9 1/5] New remove-symbol-file command. References: <1371566833-4713-1-git-send-email-nicolas.blanc@intel.com> <1371566833-4713-2-git-send-email-nicolas.blanc@intel.com> In-Reply-To: <1371566833-4713-2-git-send-email-nicolas.blanc@intel.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2013-06/txt/msg00439.txt.bz2 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 [-readnow] [-s ]* 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.