Re: [RFA 1/2] Remove some cleanups from breakpoint.c

[Simon Marchi <simon dot marchi at polymtl dot ca>]

On 2017-10-09 19:47, Tom Tromey wrote:
> @@ -9433,22 +9413,20 @@ static std::vector<symtab_and_line>
>  decode_static_tracepoint_spec (const char **arg_p)
>  {
>    VEC(static_tracepoint_marker_p) *markers = NULL;
> -  struct cleanup *old_chain;
>    const char *p = &(*arg_p)[3];
>    const char *endp;
> -  char *marker_str;
>    int i;
>
>    p = skip_spaces (p);
>
>    endp = skip_to_space (p);
>
> -  marker_str = savestring (p, endp - p);
> -  old_chain = make_cleanup (xfree, marker_str);
> +  std::string marker_str (p, endp - p);
>
> -  markers = target_static_tracepoint_markers_by_strid (marker_str);
> +  markers = target_static_tracepoint_markers_by_strid 
> (marker_str.c_str ());

It's not related to your patch, but is it possible that the markers VEC 
never gets freed?

> @@ -11761,40 +11720,36 @@ clear_command (char *arg, int from_tty)
>      }
>
>    /* Remove duplicates from the vec.  */
> -  qsort (VEC_address (breakpoint_p, found),
> -	 VEC_length (breakpoint_p, found),
> -	 sizeof (breakpoint_p),
> -	 compare_breakpoints);
> -  prev = VEC_index (breakpoint_p, found, 0);
> -  for (ix = 1; VEC_iterate (breakpoint_p, found, ix, b); ++ix)
> -    {
> -      if (b == prev)
> -	{
> -	  VEC_ordered_remove (breakpoint_p, found, ix);
> -	  --ix;
> -	}
> -    }
> +  std::sort (found.begin (), found.end (),
> +	     [=] (const breakpoint_p &a, const breakpoint_p &b)

You don't actually need to capture anything in these two lambdas.

IIUC, using "breakpoint_p" was only needed because of the VEC.  Now that 
we use std::vector, it would be nice if you changed breakpoint_p to 
breakpoint * in the code you touched.

With those changed, the patch LGTM.

Thanks,

Simon