From patchwork Thu Jan 21 10:40:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 10498 Received: (qmail 88348 invoked by alias); 21 Jan 2016 10:40:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 88337 invoked by uid 89); 21 Jan 2016 10:40:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_NONE, T_FILL_THIS_FORM_SHORT autolearn=no version=3.3.2 spammy=1387, 138, 7, 0400, H*i:sk:047d7bd X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 21 Jan 2016 10:40:07 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id C3D36117CAD; Thu, 21 Jan 2016 05:40:05 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id vgrma3ix-qsk; Thu, 21 Jan 2016 05:40:05 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id B075D1177EE; Thu, 21 Jan 2016 05:40:04 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id B351A45017; Thu, 21 Jan 2016 14:40:00 +0400 (RET) Date: Thu, 21 Jan 2016 14:40:00 +0400 From: Joel Brobecker To: Doug Evans Cc: Keith Seitz , gdb-patches@sourceware.org Subject: Re: [RFA] Fix regression introduced in "break *" by explicit location patches (was: "Re: [PATCH v6 4/9] Explicit locations: introduce address locations") Message-ID: <20160121104000.GE5146@adacore.com> References: <047d7bd91664ac4a4a0529cc80a6@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <047d7bd91664ac4a4a0529cc80a6@google.com> User-Agent: Mutt/1.5.23 (2014-03-12) > > gdb/ChangeLog: > > > > * location.h (new_address_location): Add new parameters > > "addr_string" and "addr_string_len". > > (get_address_string_location): Add declaration. > > * location.c (new_address_location): Add new parameters > > "addr_string" and "addr_string_len". If not NULL, store > > a copy of the addr_string in the new location as well. > > (get_address_string_location): New function. > > (string_to_event_location): Update call to new_address_location. > > * linespec.c (event_location_to_sals) : > > Save the event location in the parser's state before > > passing it to convert_address_location_to_sals. > > * breakpoint.c (create_thread_event_breakpoint): Update call > > to new_address_location. > > (init_breakpoint_sal): Get the event location's string, if any, > > and use it to update call to new_address_location. > > * python/py-finishbreakpoint.c (bpfinishpy_init): > > Update call to new_address_location. > > * spu-tdep.c (spu_catch_start): Likewise. > > > > * config/djgpp/fnchange.lst: Add entries for > > gdb/testsuite/gdb.base/break-fun-addr1.c and > > gdb/testsuite/gdb.base/break-fun-addr2.c. > > > > gdb/testsuite/ChangeLog: > > > > * gdb.base/break-fun-addr.exp: New file. > > * gdb.base/break-fun-addr1.c: New file. > > * gdb.base/break-fun-addr2.c: New file. > > > > Tested on x86_64-linux. > > +const char * > > +get_address_string_location (const struct event_location *location) > > +{ > > + gdb_assert (EL_TYPE (location) == ADDRESS_LOCATION); > > + return EL_STRING (location); > > +} > > + > > +/* See description in location.h. */ > > + > > Given that the argument must be an address location, > "get_address_location_string" reads better to me. I am not sure what my exact thinking was, at the time, and whether it was flawed or not, but I chose the naming at the time to be consistent with the other functions that looked similar in they functionality. The ideal name for the function would probably be something like "get_address_string_from_location", in other words, what I meant is to return the "address string" (as opposed to the address itself). For now, I pushed the patch as is, but if you think the name should be changed, here is a patch that does that. So, it's ready to go in, if you give the signal. What I would (counter-)propose, on the other hand, is that we look at renaming the function to use "_from_location". Or, maybe have the functions start with "location_" (Eg: location_get_address_string, which actually might be closer to our typical namespace strategy). From 493d020497093787700e1a055a25f8d6471ea416 Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Thu, 21 Jan 2016 14:26:33 +0400 Subject: [PATCH] rename get_address_string_location into get_address_location_string The new function name that sounds more correct. gdb/ChangeLog: * location.h, linespec.c, location.c (get_address_location_string): Renames get_address_string_location. --- gdb/ChangeLog | 5 +++++ gdb/linespec.c | 2 +- gdb/location.c | 2 +- gdb/location.h | 2 +- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index a377a32..598ac9d 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,10 @@ 2016-01-21 Joel Brobecker + * location.h, linespec.c, location.c (get_address_location_string): + Renames get_address_string_location. + +2016-01-21 Joel Brobecker + * location.h (new_address_location): Add new parameters "addr_string" and "addr_string_len". (get_address_string_location): Add declaration. diff --git a/gdb/linespec.c b/gdb/linespec.c index 2360cc1..9c8a27a 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -2513,7 +2513,7 @@ event_location_to_sals (linespec_parser *parser, case ADDRESS_LOCATION: { - const char *addr_string = get_address_string_location (location); + const char *addr_string = get_address_location_string (location); CORE_ADDR addr = get_address_location (location); if (addr_string != NULL) diff --git a/gdb/location.c b/gdb/location.c index e43ebf1..88ae61d 100644 --- a/gdb/location.c +++ b/gdb/location.c @@ -138,7 +138,7 @@ get_address_location (const struct event_location *location) /* See description in location.h. */ const char * -get_address_string_location (const struct event_location *location) +get_address_location_string (const struct event_location *location) { gdb_assert (EL_TYPE (location) == ADDRESS_LOCATION); return EL_STRING (location); diff --git a/gdb/location.h b/gdb/location.h index b2cf45e..2b04def 100644 --- a/gdb/location.h +++ b/gdb/location.h @@ -145,7 +145,7 @@ extern CORE_ADDR of the given event_location (which must be of type ADDRESS_LOCATION). */ extern const char * - get_address_string_location (const struct event_location *location); + get_address_location_string (const struct event_location *location); /* Create a new probe location. The return result is malloc'd and should be freed with delete_event_location. */ -- 2.5.0