[PATCH 0/4] [gdb] Use c++-style cleanups a bit more

[PATCH 0/4] [gdb] Use c++-style cleanups a bit more

[Tom de Vries]

I noticed a case where using a c++-style cleanup could be useful.  The first
patch fixes this.

Then I asked Claude Code if more locations need fixing.  The other three
patches fix those.

Tom de Vries (4):
  [gdb] Use make_scoped_restore in z80_read_overlay_region_table
  [gdb] Use SCOPE_EXIT in tfile_get_traceframe_address
  [gdb] Use make_scoped_restore in linespec_lexer_peek_token
  [gdb] Use make_scoped_restore in captured_main_1

 gdb/linespec.c        | 24 ++++++++++--------------
 gdb/main.c            |  7 +++----
 gdb/tracefile-tfile.c |  9 ++++++---
 gdb/z80-tdep.c        |  5 ++---
 4 files changed, 21 insertions(+), 24 deletions(-)


base-commit: f621e384d9c26125cdf71875c472fc33989d4997
-- 
2.51.0

[PATCH 1/4] [gdb] Use make_scoped_restore in z80_read_overlay_region_table

[Tom de Vries]

I came across z80_read_overlay_region_table and noticed this code:
...
   const enum overlay_debugging_state save_ovly_dbg = overlay_debugging;
   overlay_debugging = ovly_off;
   ...
   overlay_debugging = save_ovly_dbg;
...

Protect the restore of overlay_debugging against exceptions using
make_scoped_restore.
---
 gdb/z80-tdep.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/gdb/z80-tdep.c b/gdb/z80-tdep.c
index 6b995e74d9f..7822b74a78b 100644
--- a/gdb/z80-tdep.c
+++ b/gdb/z80-tdep.c
@@ -937,9 +937,9 @@ z80_read_overlay_region_table ()
       return 0;
     }
 
-  const enum overlay_debugging_state save_ovly_dbg = overlay_debugging;
   /* prevent infinite recurse */
-  overlay_debugging = ovly_off;
+  scoped_restore overlay_debugging_restore
+    = make_scoped_restore (&overlay_debugging, ovly_off);
 
   gdbarch = ovly_region_table_msym.objfile->arch ();
   word_size = gdbarch_long_bit (gdbarch) / TARGET_CHAR_BIT;
@@ -956,7 +956,6 @@ z80_read_overlay_region_table ()
 			  (unsigned int *) cache_ovly_region_table,
 			  cache_novly_regions * 3, word_size, byte_order);
 
-  overlay_debugging = save_ovly_dbg;
   return 1;                     /* SUCCESS */
 }
 
-- 
2.51.0

[PATCH 2/4] [gdb] Use SCOPE_EXIT in tfile_get_traceframe_address

[Tom de Vries]

Use SCOPE_EXIT in tfile_get_traceframe_address to protect a cleanup against
exceptions.
---
 gdb/tracefile-tfile.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 289e00ff531..83d863302f9 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -652,6 +652,12 @@ tfile_get_traceframe_address (off_t tframe_offset)
   short tpnum;
   struct tracepoint *tp;
   off_t saved_offset = cur_offset;
+  SCOPE_EXIT
+    {
+      /* Restore our seek position.  */
+      cur_offset = saved_offset;
+      lseek (trace_fd, cur_offset, SEEK_SET);
+    };
 
   /* FIXME dig pc out of collected registers.  */
 
@@ -667,9 +673,6 @@ tfile_get_traceframe_address (off_t tframe_offset)
   if (tp != nullptr && tp->has_locations ())
     addr = tp->first_loc ().address;
 
-  /* Restore our seek position.  */
-  cur_offset = saved_offset;
-  lseek (trace_fd, cur_offset, SEEK_SET);
   return addr;
 }
 
-- 
2.51.0

[PATCH 3/4] [gdb] Use make_scoped_restore in linespec_lexer_peek_token

[Tom de Vries]

Use make_scoped_restore in linespec_lexer_peek_token to protect cleanups
against exceptions.
---
 gdb/linespec.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index b6505ba283d..95ad0b048d2 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1037,20 +1037,16 @@ linespec_lexer_consume_token (linespec_parser *parser)
 static linespec_token
 linespec_lexer_peek_token (linespec_parser *parser)
 {
-  linespec_token next;
-  const char *saved_stream = parser->lexer.stream;
-  linespec_token saved_token = parser->lexer.current;
-  int saved_completion_quote_char = parser->completion_quote_char;
-  const char *saved_completion_quote_end = parser->completion_quote_end;
-  const char *saved_completion_word = parser->completion_word;
-
-  next = linespec_lexer_consume_token (parser);
-  parser->lexer.stream = saved_stream;
-  parser->lexer.current = saved_token;
-  parser->completion_quote_char = saved_completion_quote_char;
-  parser->completion_quote_end = saved_completion_quote_end;
-  parser->completion_word = saved_completion_word;
-  return next;
+  scoped_restore stream_restore = make_scoped_restore (&parser->lexer.stream);
+  scoped_restore token_restore = make_scoped_restore (&parser->lexer.current);
+  scoped_restore completion_quote_char_restore
+    = make_scoped_restore (&parser->completion_quote_char);
+  scoped_restore completion_quote_end_restore
+    = make_scoped_restore (&parser->completion_quote_end);
+  scoped_restore completion_word_restore
+    = make_scoped_restore (&parser->completion_word);
+
+  return linespec_lexer_consume_token (parser);
 }
 
 /* Helper functions.  */
-- 
2.51.0

[PATCH 4/4] [gdb] Use make_scoped_restore in captured_main_1

[Tom de Vries]

Use make_scoped_restore in captured_main_1 to protect a cleanup against
exceptions.

I don't think this is strictly necessary: exceptions that escape out of
captured_main_1 are printed, after which gdb exits, so the state of the
global_auto_load doesn't matter much at that point.

But using make_scoped_restore probably captures the intent better.

I used std::optional to avoid indenting the code in between the save and
restore.
---
 gdb/main.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index f49d5f48c9d..ea7ecc468e6 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -652,7 +652,6 @@ captured_main_1 (struct captured_main_args *context)
   std::vector<char *> dirarg;
 
   int i;
-  int save_auto_load;
   int ret = 1;
 
   /* Check for environment variables which might cause GDB to start with
@@ -1238,8 +1237,8 @@ captured_main_1 (struct captured_main_args *context)
   /* Skip auto-loading section-specified scripts until we've sourced
      local_gdbinit (which is often used to augment the source search
      path).  */
-  save_auto_load = global_auto_load;
-  global_auto_load = 0;
+  std::optional<scoped_restore_tmpl<bool>>
+    auto_load_restore (make_scoped_restore (&global_auto_load, false));
 
   if (execarg != nullptr && symarg != nullptr && streq (execarg, symarg))
     {
@@ -1333,7 +1332,7 @@ captured_main_1 (struct captured_main_args *context)
      processed, we can read any scripts mentioned in SYMARG.
      We wait until now because it is common to add to the source search
      path in local_gdbinit.  */
-  global_auto_load = save_auto_load;
+  auto_load_restore.reset ();
   for (objfile &objfile : current_program_space->objfiles ())
     load_auto_scripts_for_objfile (objfile);
 
-- 
2.51.0

Re: [PATCH 0/4] [gdb] Use c++-style cleanups a bit more

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> I noticed a case where using a c++-style cleanup could be useful.  The first
Tom> patch fixes this.

Tom> Then I asked Claude Code if more locations need fixing.  The other three
Tom> patches fix those.

These look good to me, thanks.
Approved-By: Tom Tromey <tom@tromey.com>

Tom