This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 3/9] Explicit locations v2 - Locations refactor
- From: Doug Evans <dje at google dot com>
- To: Keith Seitz <keiths at redhat dot com>
- Cc: "gdb-patches at sourceware dot org ml" <gdb-patches at sourceware dot org>
- Date: Sat, 2 Aug 2014 12:11:53 -0700
- Subject: Re: [RFA 3/9] Explicit locations v2 - Locations refactor
- Authentication-results: sourceware.org; auth=none
- References: <536BC61D dot 30700 at redhat dot com> <21361 dot 26965 dot 370300 dot 619166 at ruffy dot mtv dot corp dot google dot com> <5388CB6C dot 3080507 at redhat dot com>
On Fri, May 30, 2014 at 11:18 AM, Keith Seitz <keiths@redhat.com> wrote:
> On 05/12/2014 05:37 PM, Doug Evans wrote:
> [....]
>> > @@ -9981,7 +10036,13 @@ create_breakpoint (struct gdbarch *gdbarch,
>> > }
>> > b->cond_string = cond_string;
>> > }
>> > - b->extra_string = NULL;
>> > + if (extra_string != NULL && *extra_string != '\0')
>> > + {
>> > + b->extra_string = xstrdup (extra_string);
>> > + make_cleanup (xfree, b->extra_string);
>>
>> The cleanup here looks odd.
>
>
> Previously, b->extra_string was always set to NULL for pending breakpoints.
> extra_string is now needed for explicit locations, but this patch isn't
> dealing with this. I've moved this to the right patch (#6) [and added a
> comment]. I apologize for that.
No worries!
> [...]
>> >
>> > if (sals.nelts != 1)
>> > error (_("Couldn't get information on specified line."));
>> >
>> > + if (EVENT_LOCATION_TYPE (©_location) ==
>> EVENT_LOCATION_LINESPEC)
>> > + arg = EVENT_LOCATION_LINESPEC (©_location);
>>
>> A short comment explaining why setting arg here is necessary would help
>> me the reader.
>
>
> It's the idiom that is forced upon us by the ambiguity of linespecs. Only
> AFTER creating SaLs do we know what the user specified.
>
> You'll see this all over the place as a result.
Ah. It may take a few iterations to commit this to longer-term memory.
Apologies for the interim.