Re: [RFA] Fix off-by-one error in record.c (record_list_release_first)

Hui Zhu
2009-10-15T03:18:35Z

Thanks Michael, I think this patch is OK with me.

Hui

On Wed, Oct 14, 2009 at 02:01, Michael Snyder <msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>>
>> Thanks Michael and Jilin,
>>
>> I think the Michael's patch is more spend up.  Jilin's patch is more
>> clear and can handle set_record_insn_max_num.
>>
>> I suggest we choice speed up.  Do you think it's OK?
>>
>> If we choice it, Michael, could you please add some comments to
>> "record_list_release_first" or change it's name to
>> "record_list_release_first_without_update_xxx" to make it clear.
>> And please update set_record_insn_max_num.
>
> Hah, sorry, I completely missed the implications for
> set_record_insn_max_num.  Thanks Jilin for catching it.
>
> New diff:
>
>
> 2009-10-12  Michael Snyder  <msnyder@vmware.com>
>
>        * record.c (record_list_release_first): Do not decrement
>        record_insn_num.
>        (set_insn_num_max): Remove printf.
>        Decrement record_insn_num in the loop.
>
> Index: record.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/record.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 record.c
> --- record.c    27 Sep 2009 02:49:34 -0000      1.20
> +++ record.c    13 Oct 2009 18:04:44 -0000
> @@ -177,6 +177,11 @@ record_list_release_next (void)
>     }
>  }
>
> +/* 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_insn_num.  */
> +
>  static void
>  record_list_release_first (void)
>  {
> @@ -209,8 +214,6 @@ record_list_release_first (void)
>       if (type == record_end)
>        break;
>     }
> -
> -  record_insn_num--;
>  }
>
>  /* Add a struct record_entry to record_arch_list.  */
> @@ -1260,12 +1263,12 @@ set_record_insn_max_num (char *args, int
>  {
>   if (record_insn_num > record_insn_max_num && record_insn_max_num)
>     {
> -      printf_unfiltered (_("Record instructions number is bigger than "
> -                          "record instructions max number.  Auto delete "
> -                          "the first ones?\n"));
> -
> +      /* Count down record_insn_num while releasing records from list.  */
>       while (record_insn_num > record_insn_max_num)
> -       record_list_release_first ();
> +       {
> +         record_list_release_first ();
> +         record_insn_num--;
> +       }
>     }
>  }
>
>
>