Re: [RFA 02/12] Change breakpoints to use value_ref_ptr

[Pedro Alves <palves@redhat.com>]

On 04/05/2018 10:14 PM, Tom Tromey wrote:


>  4 files changed, 48 insertions(+), 54 deletions(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 68292626d3..67ba5a6b31 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -1740,8 +1740,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
>  	 no longer relevant.  We don't want to report a watchpoint hit
>  	 to the user when the old value and the new value may actually
>  	 be completely different objects.  */
> -      value_decref (b->val);
> -      b->val = NULL;
> +      b->val.reset (nullptr);

Just OOC, wouldn't the old "b->val = NULL;" work the same as the
reset call?

>        b->val_valid = 0;

> @@ -14533,7 +14516,7 @@ invalidate_bp_value_on_memory_change (struct inferior *inferior,
>        {
>  	struct watchpoint *wp = (struct watchpoint *) bp;
>  
> -	if (wp->val_valid && wp->val)
> +	if (wp->val_valid && wp->val.get ())

Nit, about this get(): I wonder whether we should add an
  explicit operator bool() 
implementation to gdb_ref_ptr to avoid it.

I guess we should instead write the explicit:

  if (wp->val_valid && wp->val != nullptr)

making that moot.

Thanks,
Pedro Alves