RE: [PATCH 2/2] gdb: Simplify variable set hooks

["Metzger, Markus T" <markus.t.metzger@intel.com>]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Andrew Burgess
> Sent: Friday, November 11, 2016 7:19 PM
> To: gdb-patches@sourceware.org
> Cc: Andrew Burgess <andrew.burgess@embecosm.com>
> Subject: [PATCH 2/2] gdb: Simplify variable set hooks

Hi Andrew,

> Now the the variable set-hook mechanism supports automatic rollback of
> the variable value if the set-hook throws an error, simplify existing
> cases where we manually performed roll-back within the set-hook.
> 
> gdb/ChangeLog:
> 
> 	* dcache.c (set_dcache_size): Don't change value on error path.
> 	(set_dcache_line_size): Likewise.
> 	* record.c (record_insn_history_size_setshow_var): Remove.
> 	(record_call_history_size_setshow_var): Remove.
> 	(validate_history_size): Simplify to just the error check.
> 	(set_record_insn_history_size): Update call to
> 	validate_history_size.
> 	(set_record_call_history_size): Likewise.
> 	(_initialize_record): Remove use of *_setshow_var.
> 	* symtab.c (new_symbol_cache_size): Remove.
> 	(symbol_cache_size): Update comment.
> 	(set_symbol_cache_size_handler): Simplify error check.
> 	(_initialize_symtab): Remove use of new_symbol_cache_size.
> 	* valprint.c (input_radix_1): Remove.
> 	(set_input_radix): Remove use of input_radix_1.
> 	(set_input_radix_1): Likewise.
> 	(output_radix_1): Remove.
> 	(set_output_radix): Remove use of output_radix_1.
> 	(set_output_radix_1): Likewise.
> 	(_initialize_valprint): Remove use of output_radix_1 and
> 	input_radix_1.
> 	* value.c (set_max_value_size): Simplify error case.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/max-value-size.exp: Update expected output.
> ---
>  gdb/ChangeLog                             | 25 +++++++++++++++++++
>  gdb/dcache.c                              | 12 +++------
>  gdb/record.c                              | 41 ++++++-------------------------
>  gdb/symtab.c                              | 23 ++++-------------
>  gdb/testsuite/ChangeLog                   |  4 +++
>  gdb/testsuite/gdb.base/max-value-size.exp |  4 +--
>  gdb/valprint.c                            | 31 ++++++-----------------
>  gdb/value.c                               |  6 +----
>  8 files changed, 56 insertions(+), 90 deletions(-)

The record changes look good to me.

Regarding the parent patch, if we passed the to-be-set value to the
set function in addition to the pointer to the set/show variable, we
could leave the actual assignment to the set function.  We wouldn't
need to take care of restoring the previous value in the caller since
we have not updated it, yet.  This might be simpler.

Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928