[RFA 02/22] Use RAII to save and restore scalars

Tom Tromey
2016-09-27T04:47:54Z

This patch replaces many (but not all) uses of
make_cleanup_restore_integer with a simple RAII-based template class.
It also removes the similar restore_execution_direction cleanup in
favor of this new class. Subsequent patches will replace other
similar cleanups with this class.

I chose the name "scoped_restore" for this class. I considered that
perhaps "auto_restore" might be clearer. Or maybe something else;
it's simple enough to change.

I had hoped that template parameter deduction would work here, but it
did not, and so the patch uses explicit template parameters
everywhere.

2016-09-26 Tom Tromey <tom@tromey.com>

* utils.h (class scoped_restore): New class.
* top.c (execute_command_to_string): Use scoped_restore.
* python/python.c (python_interactive_command): Use
scoped_restore.
(python_command, execute_gdb_command): Likewise.
* printcmd.c (do_one_display): Use scoped_restore.
* mi/mi-main.c (exec_continue): Use scoped_restore.
* mi/mi-cmd-var.c (mi_cmd_var_assign): Use scoped_restore.
* linux-fork.c (checkpoint_command): Use scoped_restore.
* infrun.c (restore_execution_direction): Remove.
(fetch_inferior_event): Use scoped_restore.
* compile/compile.c (compile_file_command): Use
scoped_restore.
(compile_code_command, compile_print_command): Likewise.
* cli/cli-script.c (execute_user_command): Use
scoped_restore.
(while_command, if_command, script_from_file): Likewise.
* arm-tdep.c (arm_insert_single_step_breakpoint): Use
scoped_restore.
---
gdb/ChangeLog | 22 ++++++++++++++++++++++
gdb/arm-tdep.c | 8 ++------
gdb/cli/cli-script.c | 18 ++++--------------
gdb/compile/compile.c | 23 ++++++++---------------
gdb/infrun.c | 16 ++--------------
gdb/linux-fork.c | 11 ++++++-----
gdb/mi/mi-cmd-var.c | 7 +------
gdb/mi/mi-main.c | 3 +--
gdb/printcmd.c | 5 +----
gdb/python/python.c | 12 +++---------
gdb/top.c | 3 +--
gdb/utils.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
12 files changed, 95 insertions(+), 77 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2f28e54..104048f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,27 @@
2016-09-26 Tom Tromey <tom@tromey.com>

+ * utils.h (class scoped_restore): New class.
+ * top.c (execute_command_to_string): Use scoped_restore.
+ * python/python.c (python_interactive_command): Use
+ scoped_restore.
+ (python_command, execute_gdb_command): Likewise.
+ * printcmd.c (do_one_display): Use scoped_restore.
+ * mi/mi-main.c (exec_continue): Use scoped_restore.
+ * mi/mi-cmd-var.c (mi_cmd_var_assign): Use scoped_restore.
+ * linux-fork.c (checkpoint_command): Use scoped_restore.
+ * infrun.c (restore_execution_direction): Remove.
+ (fetch_inferior_event): Use scoped_restore.
+ * compile/compile.c (compile_file_command): Use
+ scoped_restore.
+ (compile_code_command, compile_print_command): Likewise.
+ * cli/cli-script.c (execute_user_command): Use
+ scoped_restore.
+ (while_command, if_command, script_from_file): Likewise.
+ * arm-tdep.c (arm_insert_single_step_breakpoint): Use
+ scoped_restore.
+
+2016-09-26 Tom Tromey <tom@tromey.com>
+
* selftest.c: Include <vector>, not "vec.h".
(self_test_function_ptr): Remove.
(tests): Now a std::vector.
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 4dfd76b..d7a8799 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -4205,15 +4205,11 @@ arm_insert_single_step_breakpoint (struct gdbarch *gdbarch,
struct address_space *aspace,
CORE_ADDR pc)
{
- struct cleanup *old_chain
- = make_cleanup_restore_integer (&arm_override_mode);
-
- arm_override_mode = IS_THUMB_ADDR (pc);
+ scoped_restore<int> save_override_mode (&arm_override_mode,
+ IS_THUMB_ADDR (pc));
pc = gdbarch_addr_bits_remove (gdbarch, pc);

insert_single_step_breakpoint (gdbarch, aspace, pc);
-
- do_cleanups (old_chain);
}

/* Given BUF, which is OLD_LEN bytes ending at ENDADDR, expand
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 579d0a4..bf18226 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -380,8 +380,7 @@ execute_user_command (struct cmd_list_element *c, char *args)
not confused with Insight. */
in_user_command = 1;

- make_cleanup_restore_integer (&current_ui->async);
- current_ui->async = 0;
+ scoped_restore<int> save_async (&current_ui->async, 0);

command_nest_depth++;
while (cmdlines)
@@ -654,7 +653,6 @@ static void
while_command (char *arg, int from_tty)
{
struct command_line *command = NULL;
- struct cleanup *old_chain;

control_level = 1;
command = get_command_line (while_control, arg);
@@ -662,13 +660,10 @@ while_command (char *arg, int from_tty)
if (command == NULL)
return;

- old_chain = make_cleanup_restore_integer (&current_ui->async);
- current_ui->async = 0;
+ scoped_restore<int> save_async (&current_ui->async, 0);

execute_control_command_untraced (command);
free_command_lines (&command);
-
- do_cleanups (old_chain);
}

/* "if" command support. Execute either the true or false arm depending
@@ -686,13 +681,10 @@ if_command (char *arg, int from_tty)
if (command == NULL)
return;

- old_chain = make_cleanup_restore_integer (&current_ui->async);
- current_ui->async = 0;
+ scoped_restore<int> save_async (&current_ui->async, 0);

execute_control_command_untraced (command);
free_command_lines (&command);
-
- do_cleanups (old_chain);
}

/* Cleanup */
@@ -1693,10 +1685,8 @@ script_from_file (FILE *stream, const char *file)
source_line_number = 0;
source_file_name = file;

- make_cleanup_restore_integer (&current_ui->async);
- current_ui->async = 0;
-
{
+ scoped_restore<int> save_async (&current_ui->async, 0);

TRY
{
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 0c4a738..e2be115 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -91,8 +91,7 @@ compile_file_command (char *arg, int from_tty)
char *buffer;
struct cleanup *cleanup;

- cleanup = make_cleanup_restore_integer (&current_ui->async);
- current_ui->async = 0;
+ scoped_restore<int> save_async (&current_ui->async, 0);

/* Check the user did not just <enter> after command. */
if (arg == NULL)
@@ -115,7 +114,7 @@ compile_file_command (char *arg, int from_tty)

arg = skip_spaces (arg);
arg = gdb_abspath (arg);
- make_cleanup (xfree, arg);
+ cleanup = make_cleanup (xfree, arg);
buffer = xstrprintf ("#include \"%s\"\n", arg);
make_cleanup (xfree, buffer);
eval_compile_command (NULL, buffer, scope, NULL);
@@ -130,11 +129,9 @@ compile_file_command (char *arg, int from_tty)
static void
compile_code_command (char *arg, int from_tty)
{
- struct cleanup *cleanup;
enum compile_i_scope_types scope = COMPILE_I_SIMPLE_SCOPE;

- cleanup = make_cleanup_restore_integer (&current_ui->async);
- current_ui->async = 0;
+ scoped_restore<int> save_async (&current_ui->async, 0);

if (arg != NULL && check_raw_argument (&arg))
{
@@ -155,13 +152,12 @@ compile_code_command (char *arg, int from_tty)
else
{
struct command_line *l = get_command_line (compile_control, "");
+ struct cleanup *cleanup = make_cleanup_free_command_lines (&l);

- make_cleanup_free_command_lines (&l);
l->control_u.compile.scope = scope;
execute_control_command_untraced (l);
+ do_cleanups (cleanup);
}
-
- do_cleanups (cleanup);
}

/* Callback for compile_print_command. */
@@ -183,12 +179,10 @@ static void
compile_print_command (char *arg_param, int from_tty)
{
const char *arg = arg_param;
- struct cleanup *cleanup;
enum compile_i_scope_types scope = COMPILE_I_PRINT_ADDRESS_SCOPE;
struct format_data fmt;

- cleanup = make_cleanup_restore_integer (&current_ui->async);
- current_ui->async = 0;
+ scoped_restore<int> save_async (&current_ui->async, 0);

/* Passing &FMT as SCOPE_DATA is safe as do_module_cleanup will not
touch the stale pointer if compile_object_run has already quit. */
@@ -199,14 +193,13 @@ compile_print_command (char *arg_param, int from_tty)
else
{
struct command_line *l = get_command_line (compile_control, "");
+ struct cleanup *cleanup = make_cleanup_free_command_lines (&l);

- make_cleanup_free_command_lines (&l);
l->control_u.compile.scope = scope;
l->control_u.compile.scope_data = &fmt;
execute_control_command_untraced (l);
+ do_cleanups (cleanup);
}
-
- do_cleanups (cleanup);
}

/* A cleanup function to remove a directory and all its contents. */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 2636a19..7832a5d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3879,17 +3879,6 @@ all_uis_on_sync_execution_starting (void)
}
}

-/* A cleanup that restores the execution direction to the value saved
- in *ARG. */
-
-static void
-restore_execution_direction (void *arg)
-{
- enum exec_direction_kind *save_exec_dir = (enum exec_direction_kind *) arg;
-
- execution_direction = *save_exec_dir;
-}
-
/* Asynchronous version of wait_for_inferior. It is called by the
event loop whenever a change of state is detected on the file
descriptor corresponding to the target. It can be called more than
@@ -3906,7 +3895,6 @@ fetch_inferior_event (void *client_data)
struct execution_control_state *ecs = &ecss;
struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
struct cleanup *ts_old_chain;
- enum exec_direction_kind save_exec_dir = execution_direction;
int cmd_done = 0;
ptid_t waiton_ptid = minus_one_ptid;

@@ -3945,8 +3933,8 @@ fetch_inferior_event (void *client_data)
event. */
target_dcache_invalidate ();

- make_cleanup (restore_execution_direction, &save_exec_dir);
- execution_direction = target_execution_direction ();
+ scoped_restore<enum exec_direction_kind>
+ save_exec_dir (&execution_direction, target_execution_direction ());

ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws,
target_can_async_p () ? TARGET_WNOHANG : 0);
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index e7202dd..9c2cf8a 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -680,7 +680,6 @@ checkpoint_command (char *args, int from_tty)
struct value *fork_fn = NULL, *ret;
struct fork_info *fp;
pid_t retpid;
- struct cleanup *old_chain;

if (!target_has_execution)
error (_("The program is not being run."));
@@ -704,11 +703,13 @@ checkpoint_command (char *args, int from_tty)
ret = value_from_longest (builtin_type (gdbarch)->builtin_int, 0);

/* Tell linux-nat.c that we're checkpointing this inferior. */
- old_chain = make_cleanup_restore_integer (&checkpointing_pid);
- checkpointing_pid = ptid_get_pid (inferior_ptid);
+ {
+ scoped_restore<int> save_pid (&checkpointing_pid,
+ ptid_get_pid (inferior_ptid));
+
+ ret = call_function_by_hand (fork_fn, 0, &ret);
+ }

- ret = call_function_by_hand (fork_fn, 0, &ret);
- do_cleanups (old_chain);
if (!ret) /* Probably can't happen. */
error (_("checkpoint: call_function_by_hand returned null."));

diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 3bfe4f0..1c78104 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -608,7 +608,6 @@ mi_cmd_var_assign (char *command, char **argv, int argc)
struct ui_out *uiout = current_uiout;
struct varobj *var;
char *expression, *val;
- struct cleanup *cleanup;

if (argc != 2)
error (_("-var-assign: Usage: NAME EXPRESSION."));
@@ -623,9 +622,7 @@ mi_cmd_var_assign (char *command, char **argv, int argc)

/* MI command '-var-assign' may write memory, so suppress memory
changed notification if it does. */
- cleanup
- = make_cleanup_restore_integer (&mi_suppress_notification.memory);
- mi_suppress_notification.memory = 1;
+ scoped_restore<int> save_suppress (&mi_suppress_notification.memory, 1);

if (!varobj_set_value (var, expression))
error (_("-var-assign: Could not assign "
@@ -634,8 +631,6 @@ mi_cmd_var_assign (char *command, char **argv, int argc)
val = varobj_get_value (var);
ui_out_field_string (uiout, "value", val);
xfree (val);
-
- do_cleanups (cleanup);
}

/* Type used for parameters passing to mi_cmd_var_update_iter. */
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 1913157..c6c7067 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -300,7 +300,7 @@ exec_continue (char **argv, int argc)
}
else
{
- struct cleanup *back_to = make_cleanup_restore_integer (&sched_multi);
+ scoped_restore<int> save_multi (&sched_multi);

if (current_context->all)
{
@@ -315,7 +315,6 @@ exec_continue (char **argv, int argc)
same. */
continue_1 (1);
}
- do_cleanups (back_to);
}
}

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index d4a4b9e..1fccd26 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1938,7 +1938,6 @@ undisplay_command (char *args, int from_tty)
static void
do_one_display (struct display *d)
{
- struct cleanup *old_chain;
int within_current_scope;

if (d->enabled_p == 0)
@@ -1990,8 +1989,7 @@ do_one_display (struct display *d)
if (!within_current_scope)
return;

- old_chain = make_cleanup_restore_integer (&current_display_number);
- current_display_number = d->number;
+ scoped_restore<int> save_display_number (&current_display_number, d->number);

annotate_display_begin ();
printf_filtered ("%d", d->number);
@@ -2079,7 +2077,6 @@ do_one_display (struct display *d)
annotate_display_end ();

gdb_flush (gdb_stdout);
- do_cleanups (old_chain);
}

/* Display all of the values on the auto-display chain which can be
diff --git a/gdb/python/python.c b/gdb/python/python.c
index b00b70b..9e4d610 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -319,11 +319,9 @@ static void
python_interactive_command (char *arg, int from_tty)
{
struct ui *ui = current_ui;
- struct cleanup *cleanup;
int err;

- cleanup = make_cleanup_restore_integer (&current_ui->async);
- current_ui->async = 0;
+ scoped_restore<int> save_async (&current_ui->async, 0);

arg = skip_spaces (arg);

@@ -351,8 +349,6 @@ python_interactive_command (char *arg, int from_tty)
gdbpy_print_stack ();
error (_("Error while executing Python code."));
}
-
- do_cleanups (cleanup);
}

/* A wrapper around PyRun_SimpleFile. FILE is the Python script to run
@@ -467,8 +463,7 @@ python_command (char *arg, int from_tty)

cleanup = ensure_python_env (get_current_arch (), current_language);

- make_cleanup_restore_integer (&current_ui->async);
- current_ui->async = 0;
+ scoped_restore<int> save_async (&current_ui->async, 0);

arg = skip_spaces (arg);
if (arg && *arg)
@@ -651,8 +646,7 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
struct cleanup *cleanup = make_cleanup (xfree, copy);
struct interp *interp;

- make_cleanup_restore_integer (&current_ui->async);
- current_ui->async = 0;
+ scoped_restore<int> save_async (&current_ui->async, 0);

make_cleanup_restore_current_uiout ();

diff --git a/gdb/top.c b/gdb/top.c
index 3cfa113..84285b2 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -701,8 +701,7 @@ execute_command_to_string (char *p, int from_tty)
restoration callbacks. */
cleanup = set_batch_flag_and_make_cleanup_restore_page_info ();

- make_cleanup_restore_integer (&current_ui->async);
- current_ui->async = 0;
+ scoped_restore<int> save_async (&current_ui->async, 0);

str_file = mem_fileopen ();

diff --git a/gdb/utils.h b/gdb/utils.h
index 8635075..63583ed 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -84,6 +84,50 @@ extern struct cleanup *make_cleanup_bfd_unref (bfd *abfd);
struct obstack;
extern struct cleanup *make_cleanup_obstack_free (struct obstack *obstack);

+// An RAII-based object that saves a variable's value, and then
+// restores it again when this object is destroyed.
+template<typename T>
+class scoped_restore
+{
+ public:
+
+ // Create a new scoped_restore object that saves the current value
+ // of *VAR, and then sets *VAR to VALUE. *VAR will be restored when
+ // this scoped_restore object is destroyed.
+ scoped_restore (T *var, T value)
+ : saved_var (var),
+ saved_value (*var)
+ {
+ *var = value;
+ }
+
+ // Create a new scoped_restore object that saves the current value
+ // of *VAR. *VAR will be restored when this scoped_restore object
+ // is destroyed.
+ explicit scoped_restore (T *var)
+ : saved_var (var),
+ saved_value (*var)
+ {
+ }
+
+ ~scoped_restore ()
+ {
+ *saved_var = saved_value;
+ }
+
+ private:
+
+ // No need for these. They are intentionally not defined anywhere.
+ scoped_restore &operator= (const scoped_restore &);
+ scoped_restore (const scoped_restore &);
+
+ // The saved variable.
+ T *saved_var;
+
+ // The saved value.
+ T saved_value;
+};
+
extern struct cleanup *make_cleanup_restore_integer (int *variable);
extern struct cleanup *make_cleanup_restore_uinteger (unsigned int *variable);

--
2.7.4