Re: [PATCH 5/5] Don't memset non-POD types: struct breakpoint

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

On 2017-04-12 22:27, Pedro Alves wrote:
> Eh, struct breakpoint was made non-POD just today, with commit
> d28cd78ad820e3 ("Change breakpoint event locations to
> event_location_up").  :-)
>
>   src/gdb/breakpoint.c: In function ‘void
> init_raw_breakpoint_without_location(breakpoint*, gdbarch*, bptype,
> const breakpoint_ops*)’:
>   src/gdb/breakpoint.c:7447:28: error: use of deleted function ‘void*
> memset(T*, int, size_t) [with T = breakpoint; <template-parameter-1-2>
> = void; size_t = long unsigned int]’
>      memset (b, 0, sizeof (*b));
> 			      ^
>   In file included from src/gdb/common/common-defs.h:85:0,
> 		   from src/gdb/defs.h:28,
> 		   from src/gdb/breakpoint.c:20:
>   src/gdb/common/poison.h:56:7: note: declared here
>    void *memset (T *s, int c, size_t n) = delete;
> 	 ^
>
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>
> 	* breakpoint.h (struct breakpoint): In-class initialize all
> 	fields.  Make boolean fields "bool".
> 	* breakpoint.c (init_raw_breakpoint_without_location): Remove
> 	memset call and initializations no longer necessary.
> ---
>  gdb/breakpoint.c | 10 ----------
>  gdb/breakpoint.h | 60 
> ++++++++++++++++++++++++++++----------------------------
>  2 files changed, 30 insertions(+), 40 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 0796313..0e6aecc 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -7444,8 +7444,6 @@ init_raw_breakpoint_without_location (struct
> breakpoint *b,
>  				      enum bptype bptype,
>  				      const struct breakpoint_ops *ops)
>  {
> -  memset (b, 0, sizeof (*b));
> -
>    gdb_assert (ops != NULL);
>
>    b->ops = ops;
> @@ -7453,17 +7451,9 @@ init_raw_breakpoint_without_location (struct
> breakpoint *b,
>    b->gdbarch = gdbarch;
>    b->language = current_language->la_language;
>    b->input_radix = input_radix;
> -  b->thread = -1;
>    b->enable_state = bp_enabled;

Is there a reason you don't assign bp_enabled in-class directly?

> -  b->next = 0;
> -  b->silent = 0;
> -  b->ignore_count = 0;
> -  b->commands = NULL;
>    b->frame_id = null_frame_id;

I think you can remove the assignment to frame_id, since it's done 
in-class.

> -  b->condition_not_parsed = 0;
> -  b->py_bp_object = NULL;
>    b->related_breakpoint = b;
> -  b->location = NULL;
>  }
>
>  /* Helper to set_raw_breakpoint below.  Creates a breakpoint
> diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
> index 18b284f..ae84349 100644
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -681,45 +681,45 @@ extern int target_exact_watchpoints;
>  struct breakpoint
>  {
>    /* Methods associated with this breakpoint.  */
> -  const struct breakpoint_ops *ops;
> +  const breakpoint_ops *ops = NULL;
>
> -  struct breakpoint *next;
> +  breakpoint *next = NULL;
>    /* Type of breakpoint.  */
> -  enum bptype type;
> +  bptype type {bp_none};

For consistency, I think it would be nice to use = when possible

  bptype type = bp_none;