RE: [PATCH v3 2/7] gdb/record: remove record_full_insn_num

["Schimpe, Christina" <christina.schimpe@intel.com>]

> -----Original Message-----
> From: Guinevere Larsen <guinevere@redhat.com>
> Sent: Freitag, 15. Mai 2026 18:37
> To: gdb-patches@sourceware.org
> Cc: Guinevere Larsen <guinevere@redhat.com>; Thiago Jung Bauermann
> <thiago.bauermann@linaro.org>
> Subject: [PATCH v3 2/7] gdb/record: remove record_full_insn_num
> 
> Now that record_full_list uses a deque, we don't need a variable to track the
> amount of items recorded, so it is just dropped.
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> ---
>  gdb/record-full.c | 41 ++++++++++++-----------------------------
>  1 file changed, 12 insertions(+), 29 deletions(-)
> 
> diff --git a/gdb/record-full.c b/gdb/record-full.c index
> 02a57726a95..e4dd07dc300 100644
> --- a/gdb/record-full.c
> +++ b/gdb/record-full.c
> @@ -193,8 +193,6 @@ static bool record_full_stop_at_limit = true;
>  /* Maximum allowed number of insns in execution log.  */  static unsigned int
> record_full_insn_max_num
>  	= DEFAULT_RECORD_FULL_INSN_MAX_NUM;
> -/* Actual count of insns presently in execution log.  */ -static unsigned int
> record_full_insn_num = 0;
>  /* Count of insns logged so far (may be larger
>     than count of insns presently in execution log).  */  static ULONGEST
> record_full_insn_count; @@ -462,7 +460,6 @@ record_full_entry_cleanup
> (record_full_entry rec)  static void  record_full_reset_history ()  {
> -  record_full_insn_num = 0;
>    record_full_insn_count = 0;
>    record_full_next_insn = 0;
> 
> @@ -504,9 +501,7 @@ record_full_save_instruction ()  }
> 
>  /* Delete the first instruction from the beginning of the log, to make
> -   room for adding a new instruction at the end of the log.
> -
> -   Note -- this function does not modify record_full_insn_num.  */
> +   room for adding a new instruction at the end of the log.  */
> 
>  static void
>  record_full_list_release_first (void)
> @@ -606,7 +601,7 @@ record_full_arch_list_add_mem (CORE_ADDR addr,
> int len)  static void  record_full_check_insn_num (void)  {
> -  if (record_full_insn_num == record_full_insn_max_num)
> +  if (record_full_list.size () == record_full_insn_max_num)
>      {
>        /* Ask user what to do.  */
>        if (record_full_stop_at_limit)
> @@ -688,10 +683,8 @@ record_full_message (struct regcache *regcache,
> enum gdb_signal signal)
> 
>    record_full_save_instruction ();
> 
> -  if (record_full_insn_num == record_full_insn_max_num)
> +  if (record_full_list.size () == record_full_insn_max_num)
>      record_full_list_release_first ();
> -  else
> -    record_full_insn_num++;
>  }
> 
>  static bool
> @@ -1468,10 +1461,8 @@ record_full_registers_change (struct regcache
> *regcache, int regnum)
>      }
>    record_full_save_instruction ();
> 
> -  if (record_full_insn_num == record_full_insn_max_num)
> +  if (record_full_list.size () == record_full_insn_max_num)
>      record_full_list_release_first ();
> -  else
> -    record_full_insn_num++;
>  }
> 
>  /* "store_registers" method for process record target.  */ @@ -1572,10
> +1563,8 @@ record_full_target::xfer_partial (enum target_object object,
>  	}
>        record_full_save_instruction ();
> 
> -      if (record_full_insn_num == record_full_insn_max_num)
> +      if (record_full_list.size () == record_full_insn_max_num)
>  	record_full_list_release_first ();
> -      else
> -	record_full_insn_num++;
>      }
> 
>    return this->beneath ()->xfer_partial (object, annex, readbuf, writebuf, @@ -
> 1813,8 +1802,8 @@ record_full_base_target::info_record ()
>  		  pulongest (record_full_insn_count));
> 
>        /* Display log count.  */
> -      gdb_printf (_("Log contains %u instructions.\n"),
> -		  record_full_insn_num);
> +      gdb_printf (_("Log contains %lu instructions.\n"),
> +		  (unsigned long int) record_full_list.size ());
>      }
>    else
>      gdb_printf (_("No instructions have been logged.\n")); @@ -2296,8 +2285,6
> @@ record_full_restore (struct bfd &cbfd)
>  		"RECORD_FULL_FILE_MAGIC (0x%s)\n",
>  		phex_nz (netorder32 (magic), 4));
> 
> -  record_full_insn_num = 0;
> -
>    try
>      {
>        while (bfd_offset < osec_size)
> @@ -2338,9 +2325,9 @@ record_full_restore (struct bfd &cbfd)
>      }
> 
>    /* Update record_full_insn_max_num.  */
> -  if (record_full_insn_num > record_full_insn_max_num)
> +  if (record_full_list.size () > record_full_insn_max_num)
>      {
> -      record_full_insn_max_num = record_full_insn_num;
> +      record_full_insn_max_num = record_full_list.size ();
>        warning (_("Auto increase record/replay buffer limit to %u."),
>  	       record_full_insn_max_num);
>      }
> @@ -2595,14 +2582,10 @@ static void
>  set_record_full_insn_max_num (const char *args, int from_tty,
>  			      struct cmd_list_element *c)
>  {
> -  if (record_full_insn_num > record_full_insn_max_num)
> +  if (record_full_list.size () > record_full_insn_max_num)
>      {
> -      /* Count down record_full_insn_num while releasing records from list.  */
> -      while (record_full_insn_num > record_full_insn_max_num)
> -       {
> -	 record_full_list_release_first ();
> -	 record_full_insn_num--;
> -       }
> +      while (record_full_list.size () > record_full_insn_max_num)
> +	record_full_list_release_first ();
>      }
>  }
> 
> --
> 2.54.0
> 


LGTM.

Reviewed-By: Christina Schimpe <christina.schimpe@intel.com>

Christina


Intel Deutschland GmbH
Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928