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

[Hui Zhu <teawater at gmail dot com>]

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--;
> + ? ? ? }
> ? ? }
> ?}
>
>
>