Re: [RFA 5/9] Explicit locations v2 - Add probe locations

[Doug Evans <dje at google dot com>]

On Sat, Aug 2, 2014 at 4:38 PM, Doug Evans <dje@google.com> wrote:
> [...]
>  > diff --git a/gdb/probe.c b/gdb/probe.c
>  > index c7597d9..2ff5d86 100644
>  > --- a/gdb/probe.c
>  > +++ b/gdb/probe.c
>  > @@ -58,7 +58,8 @@ parse_probes (struct event_location *location,
>  >    result.sals = NULL;
>  >    result.nelts = 0;
>  >
>  > -  arg_start = EVENT_LOCATION_LINESPEC (location);
>  > +  gdb_assert (EVENT_LOCATION_TYPE (location) == PROBE_LOCATION);
>  > +  arg_start = EVENT_LOCATION_PROBE (location);
>  >
>  >    cs = arg_start;
>  >    probe_ops = probe_linespec_to_ops (&cs);
>  > @@ -173,12 +174,12 @@ parse_probes (struct event_location *location,
>  >      {
>  >        canonical->special_display = 1;
>  >        canonical->pre_expanded = 1;
>  > -      canonical->location = new_linespec_location (NULL);
>  > -      EVENT_LOCATION_LINESPEC (canonical->location)
>  > +      canonical->location = new_probe_location (NULL);
>  > +      EVENT_LOCATION_PROBE (canonical->location)
>  >      = savestring (arg_start, arg_end - arg_start);
>
> I see why you pass NULL to new_probe_location and then set EVENT_LOCATION_PROBE
> afterwards (otherwise two copies of the string would be malloc'd, and you'd
> need to deal with freeing one of them).  One could add a version of
> new_probe_location that took a char* and a length, but that seems excessive.
> Another place where c++ would allow cleaner code.

OTOH, is that, or any other variation, excessive?
It's preferable to not have users of object foo know how to physically
construct a foo.

/just a thought for discussion's sake - I wouldn't change the patch if
this is but one of many similar instances, unless it makes sense to
change all of them.