Re: [RFA] Remove VEC from breakpoint

[Simon Marchi <simark@simark.ca>]

On 2018-07-03 10:33 AM, Andrew Burgess wrote:
> * Tom Tromey <tom@tromey.com> [2018-06-05 13:23:16 -0600]:
> 
>> This removes a use of VEC from breakpoint.h, also removing the
>> now-unnecessary breakpoint_p typedef.
>>
>> This patch fixes a latent memory leak in
>> find_matching_tracepoint_location, which neglected to free the vector
>> returned by all_tracepoints.
>>
>> Tested by the buildbot.
>>
>> gdb/ChangeLog
>> 2018-06-05  Tom Tromey  <tom@tromey.com>
>>
>> 	* tracepoint.c (process_tracepoint_on_disconnect, start_tracing)
>> 	(stop_tracing, tstatus_command)
>> 	(find_matching_tracepoint_location, merge_uploaded_tracepoints)
>> 	(print_one_static_tracepoint_marker): Update.
>> 	* breakpoint.c (static_tracepoints_here, all_tracepoints): Return
>> 	std::vector.
>> 	* breakpoint.h (breakpoint_p): Remove typedef.  Don't declare
>> 	VEC.
>> 	(all_tracepoints, static_tracepoints_here): Return std::vector.
> 
> FWIW this looks fine to me, except for one nit below...

[snip]

>> @@ -3094,9 +3063,8 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
>>  {
>>    struct uploaded_tp *utp;
>>    /* A set of tracepoints which are modified.  */
>> -  VEC(breakpoint_p) *modified_tp = NULL;
>> +  std::vector<breakpoint *> modified_tp;
>>    int ix;
>> -  struct breakpoint *b;
> 
> The 'ix' is no longer used and could be deleted.
> 
> While I was double checking this I also spotted another 'ix' in
> merge_uploaded_trace_state_variables that's not used... :)

Agreed, the patch LGTM in any case.  Compiling tracepoint.c with -Wunused shows a
few more:


/home/simark/src/binutils-gdb/gdb/tracepoint.c:1546:32: error: unused variable 'tsv' [-Werror,-Wunused-variable]
  struct trace_state_variable *tsv;
                               ^
/home/simark/src/binutils-gdb/gdb/tracepoint.c:2755:28: error: unused variable 'default_collect_action' [-Werror,-Wunused-variable]
      struct command_line *default_collect_action;
                           ^
/home/simark/src/binutils-gdb/gdb/tracepoint.c:3067:7: error: unused variable 'ix' [-Werror,-Wunused-variable]
  int ix;
      ^
/home/simark/src/binutils-gdb/gdb/tracepoint.c:3183:7: error: unused variable 'ix' [-Werror,-Wunused-variable]
  int ix;
      ^
/home/simark/src/binutils-gdb/gdb/tracepoint.c:3686:26: error: unused variable 'b' [-Werror,-Wunused-variable]
      struct breakpoint *b;
                         ^

It looks like an obvious patch to clean these up is in order, so it doesn't really
matter if this patch or an eventual cleanup patch removes these ix variables.

Thanks!

Simon