From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8950 invoked by alias); 13 Oct 2009 06:46:33 -0000 Received: (qmail 8907 invoked by uid 22791); 13 Oct 2009 06:46:30 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail-px0-f202.google.com (HELO mail-px0-f202.google.com) (209.85.216.202) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 13 Oct 2009 06:46:26 +0000 Received: by pxi40 with SMTP id 40so10474550pxi.24 for ; Mon, 12 Oct 2009 23:46:25 -0700 (PDT) MIME-Version: 1.0 Received: by 10.142.9.37 with SMTP id 37mr550071wfi.116.1255416385166; Mon, 12 Oct 2009 23:46:25 -0700 (PDT) In-Reply-To: <7d77a27d0910121917w4abfc09cx43e5667393ef9f79@mail.gmail.com> References: <4AD35AB6.2050903@vmware.com> <7d77a27d0910121917w4abfc09cx43e5667393ef9f79@mail.gmail.com> From: Hui Zhu Date: Tue, 13 Oct 2009 06:46:00 -0000 Message-ID: Subject: Re: [RFA] Fix off-by-one error in record.c (record_list_release_first) To: Jiang Jilin , Michael Snyder Cc: "gdb-patches@sourceware.org" Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-10/txt/msg00260.txt.bz2 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. Thanks, Hui On Tue, Oct 13, 2009 at 10:17, Jiang Jilin wrote: > On Tue, Oct 13, 2009 at 12:35 AM, Michael Snyder wro= te: >> Hui, >> >> My "info record" patch helped me to find a bug. >> I found that, once we start calling record_list_release_first, >> we start adding two instructions to the log for every one >> instruction we remove. =A0Therefore the log continues to grow, >> even though it is supposed to remain constant in size. >> >> This is because record_insn_num is not incremented if we >> call record_list_release_first -- but record_list_release_first >> does decrement it. > > Hi Michael, > > I've noticed this bug(maybe) earlier, but I prefer fixing the caller > functions, not the callee function record_list_release_first. And > your patch doesn't handle set_record_insn_max_num. > > Thanks! > > 2009-10-13 =A0Michael Snyder =A0 > =A0 =A0 =A0 =A0Jiang Jilin =A0 > > =A0 =A0 =A0 =A0* record.c (record_message): Increase record_insn_num after > =A0 =A0 =A0 =A0record_list_release_first. > =A0 =A0 =A0 =A0(record_registers_change): Ditto. > =A0 =A0 =A0 =A0(record_xfer_partial): Ditto. > > diff --git a/gdb/record.c b/gdb/record.c > index 8b56010..0208dac 100644 > --- a/gdb/record.c > +++ b/gdb/record.c > @@ -438,8 +438,8 @@ record_message (void *args) > > =A0 if (record_insn_num =3D=3D record_insn_max_num && record_insn_max_num) > =A0 =A0 record_list_release_first (); > - =A0else > - =A0 =A0record_insn_num++; > + > + =A0record_insn_num++; > > =A0 return 1; > =A0} > @@ -1008,8 +1008,8 @@ record_registers_change (struct regcache > *regcache, int regnum) > > =A0 if (record_insn_num =3D=3D record_insn_max_num && record_insn_max_num) > =A0 =A0 record_list_release_first (); > - =A0else > - =A0 =A0record_insn_num++; > + > + =A0record_insn_num++; > =A0} > > =A0static void > @@ -1121,8 +1121,8 @@ record_xfer_partial (struct target_ops *ops, > enum target_object object, > > =A0 =A0 =A0 if (record_insn_num =3D=3D record_insn_max_num && record_insn= _max_num) > =A0 =A0 =A0 =A0record_list_release_first (); > - =A0 =A0 =A0else > - =A0 =A0 =A0 record_insn_num++; > + > + =A0 =A0 =A0record_insn_num++; > =A0 =A0 } > > =A0 return record_beneath_to_xfer_partial (record_beneath_to_xfer_partial= _ops, >