From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-67735-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Received: (qmail 31578 invoked by alias); 16 Oct 2009 02:52:14 -0000
Received: (qmail 31560 invoked by uid 22791); 16 Oct 2009 02:52:12 -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-f188.google.com (HELO mail-px0-f188.google.com) (209.85.216.188)     by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 16 Oct 2009 02:52:07 +0000
Received: by pxi26 with SMTP id 26so1584019pxi.25         for <gdb-patches@sourceware.org>; Thu, 15 Oct 2009 19:52:06 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.142.248.42 with SMTP id v42mr79186wfh.187.1255661526125; Thu,  	15 Oct 2009 19:52:06 -0700 (PDT)
In-Reply-To: <4AD75788.9050409@vmware.com>
References: <4AD367DC.9020901@vmware.com> <20091012231232.GH5272@adacore.com>  	<daef60380910142342q7c17311frcad08867d48e0b7b@mail.gmail.com>  	<4AD75788.9050409@vmware.com>
From: Hui Zhu <teawater@gmail.com>
Date: Fri, 16 Oct 2009 02:52:00 -0000
Message-ID: <daef60380910151951q68e8962fx8d792e551ee4dc24@mail.gmail.com>
Subject: Re: [RFA] Structure and simplify record log in record.c
To: Michael Snyder <msnyder@vmware.com>
Cc: Joel Brobecker <brobecker@adacore.com>,  	"gdb-patches@sourceware.org" <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: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
X-SW-Source: 2009-10/txt/msg00358.txt.bz2

Thanks Michael.

Hui

On Fri, Oct 16, 2009 at 01:10, Michael Snyder <msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>>
>> Thanks Michaael and Joel,
>>
>> I think the other part of this patch is OK with me.
>>
>> But you still has 2 patches "[RFA] Fix off-by-one error in record.c
>> (record_list_release_first)" and "[RFA] Expand "info record" will
>> change the code of record list.
>> So maybe this patch need some update according to these patches.
>
> Don't worry -- it's my responsibility to keep it in sync.
> Committed as below, with Joel's suggestions and sync
> (test suites passed on x86).
>
>
>
> 2009-10-15 =A0Michael Snyder =A0<msnyder@vmware.com>
>
> =A0 =A0 =A0 =A0* record.c (record_reg_alloc): New function.
> =A0 =A0 =A0 =A0(record_reg_release): New function.
> =A0 =A0 =A0 =A0(record_mem_alloc): New function.
> =A0 =A0 =A0 =A0(record_mem_release): New function.
> =A0 =A0 =A0 =A0(record_end_alloc): New function.
> =A0 =A0 =A0 =A0(record_end_release): New function.
> =A0 =A0 =A0 =A0(record_entry_release): New function.
> =A0 =A0 =A0 =A0(record_list_release): Simplify, call record_entry_release.
> =A0 =A0 =A0 =A0(record_list_release_next): Rename to record_list_release_=
following.
> =A0 =A0 =A0 =A0Simplify and call record_entry_release.
> =A0 =A0 =A0 =A0(record_list_release_first): Simplify, comment, and use
> =A0 =A0 =A0 =A0record_entry_release.
> =A0 =A0 =A0 =A0(record_arch_list_add_reg): Simplify, call record_reg_allo=
c.
> =A0 =A0 =A0 =A0(record_arch_list_add_mem): Simplify, call record_mem_allo=
c.
> =A0 =A0 =A0 =A0(record_arch_list_add_end): Simplify, call record_end_allo=
c.
>
> Index: record.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> RCS file: /cvs/src/src/gdb/record.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 record.c
> --- record.c =A0 =A015 Oct 2009 16:57:36 -0000 =A0 =A0 =A01.21
> +++ record.c =A0 =A015 Oct 2009 17:14:30 -0000
> @@ -129,50 +129,143 @@ static int (*record_beneath_to_insert_br
> =A0static int (*record_beneath_to_remove_breakpoint) (struct gdbarch *,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 struct bp_target_info *);
>
> +/* Alloc and free functions for record_reg, record_mem, and record_end
> + =A0 entries. =A0*/
> +
> +/* Alloc a record_reg record entry. =A0*/
> +
> +static inline struct record_entry *
> +record_reg_alloc (struct regcache *regcache, int regnum)
> +{
> + =A0struct record_entry *rec;
> + =A0struct gdbarch *gdbarch =3D get_regcache_arch (regcache);
> +
> + =A0rec =3D (struct record_entry *) xcalloc (1, sizeof (struct record_en=
try));
> + =A0rec->type =3D record_reg;
> + =A0rec->u.reg.num =3D regnum;
> + =A0rec->u.reg.val =3D (gdb_byte *) xmalloc (MAX_REGISTER_SIZE);
> +
> + =A0return rec;
> +}
> +
> +/* Free a record_reg record entry. =A0*/
> +
> +static inline void
> +record_reg_release (struct record_entry *rec)
> +{
> + =A0gdb_assert (rec->type =3D=3D record_reg);
> + =A0xfree (rec->u.reg.val);
> + =A0xfree (rec);
> +}
> +
> +/* Alloc a record_mem record entry. =A0*/
> +
> +static inline struct record_entry *
> +record_mem_alloc (CORE_ADDR addr, int len)
> +{
> + =A0struct record_entry *rec;
> +
> + =A0rec =3D (struct record_entry *) xcalloc (1, sizeof (struct record_en=
try));
> + =A0rec->type =3D record_mem;
> + =A0rec->u.mem.addr =3D addr;
> + =A0rec->u.mem.len =3D len;
> + =A0rec->u.mem.val =3D (gdb_byte *) xmalloc (len);
> +
> + =A0return rec;
> +}
> +
> +/* Free a record_mem record entry. =A0*/
> +
> +static inline void
> +record_mem_release (struct record_entry *rec)
> +{
> + =A0gdb_assert (rec->type =3D=3D record_mem);
> + =A0xfree (rec->u.mem.val);
> + =A0xfree (rec);
> +}
> +
> +/* Alloc a record_end record entry. =A0*/
> +
> +static inline struct record_entry *
> +record_end_alloc (void)
> +{
> + =A0struct record_entry *rec;
> +
> + =A0rec =3D (struct record_entry *) xcalloc (1, sizeof (struct record_en=
try));
> + =A0rec->type =3D record_end;
> +
> + =A0return rec;
> +}
> +
> +/* Free a record_end record entry. =A0*/
> +
> +static inline void
> +record_end_release (struct record_entry *rec)
> +{
> + =A0xfree (rec);
> +}
> +
> +/* Free one record entry, any type.
> + =A0 Return entry->type, in case caller wants to know. =A0*/
> +
> +static inline enum record_type
> +record_entry_release (struct record_entry *rec)
> +{
> + =A0enum record_type type =3D rec->type;
> +
> + =A0switch (type) {
> + =A0case record_reg:
> + =A0 =A0record_reg_release (rec);
> + =A0 =A0break;
> + =A0case record_mem:
> + =A0 =A0record_mem_release (rec);
> + =A0 =A0break;
> + =A0case record_end:
> + =A0 =A0record_end_release (rec);
> + =A0 =A0break;
> + =A0}
> + =A0return type;
> +}
> +
> +/* Free all record entries in list pointed to by REC. =A0*/
> +
> =A0static void
> =A0record_list_release (struct record_entry *rec)
> =A0{
> - =A0struct record_entry *tmp;
> -
> =A0 if (!rec)
> =A0 =A0 return;
>
> =A0 while (rec->next)
> - =A0 =A0{
> - =A0 =A0 =A0rec =3D rec->next;
> - =A0 =A0}
> + =A0 =A0rec =3D rec->next;
>
> =A0 while (rec->prev)
> =A0 =A0 {
> - =A0 =A0 =A0tmp =3D rec;
> =A0 =A0 =A0 rec =3D rec->prev;
> - =A0 =A0 =A0if (tmp->type =3D=3D record_reg)
> - =A0 =A0 =A0 xfree (tmp->u.reg.val);
> - =A0 =A0 =A0else if (tmp->type =3D=3D record_mem)
> - =A0 =A0 =A0 xfree (tmp->u.mem.val);
> - =A0 =A0 =A0xfree (tmp);
> + =A0 =A0 =A0record_entry_release (rec->next);
> =A0 =A0 }
>
> - =A0if (rec !=3D &record_first)
> - =A0 =A0xfree (rec);
> + =A0if (rec =3D=3D &record_first)
> + =A0 =A0{
> + =A0 =A0 =A0record_insn_num =3D 0;
> + =A0 =A0 =A0record_first.next =3D NULL;
> + =A0 =A0}
> + =A0else
> + =A0 =A0record_entry_release (rec);
> =A0}
>
> +/* Free all record entries forward of the given list position. =A0*/
> +
> =A0static void
> -record_list_release_next (void)
> +record_list_release_following (struct record_entry *rec)
> =A0{
> - =A0struct record_entry *rec =3D record_list;
> =A0 struct record_entry *tmp =3D rec->next;
> +
> =A0 rec->next =3D NULL;
> =A0 while (tmp)
> =A0 =A0 {
> =A0 =A0 =A0 rec =3D tmp->next;
> - =A0 =A0 =A0if (tmp->type =3D=3D record_end)
> + =A0 =A0 =A0if (record_entry_release (tmp) =3D=3D record_end)
> =A0 =A0 =A0 =A0record_insn_num--;
> - =A0 =A0 =A0else if (tmp->type =3D=3D record_reg)
> - =A0 =A0 =A0 xfree (tmp->u.reg.val);
> - =A0 =A0 =A0else if (tmp->type =3D=3D record_mem)
> - =A0 =A0 =A0 xfree (tmp->u.mem.val);
> - =A0 =A0 =A0xfree (tmp);
> =A0 =A0 =A0 tmp =3D rec;
> =A0 =A0 }
> =A0}
> @@ -185,34 +278,31 @@ record_list_release_next (void)
> =A0static void
> =A0record_list_release_first (void)
> =A0{
> - =A0struct record_entry *tmp =3D NULL;
> - =A0enum record_type type;
> + =A0struct record_entry *tmp;
>
> =A0 if (!record_first.next)
> =A0 =A0 return;
>
> + =A0/* Loop until a record_end. =A0*/
> =A0 while (1)
> =A0 =A0 {
> - =A0 =A0 =A0type =3D record_first.next->type;
> -
> - =A0 =A0 =A0if (type =3D=3D record_reg)
> - =A0 =A0 =A0 xfree (record_first.next->u.reg.val);
> - =A0 =A0 =A0else if (type =3D=3D record_mem)
> - =A0 =A0 =A0 xfree (record_first.next->u.mem.val);
> + =A0 =A0 =A0/* Cut record_first.next out of the linked list. =A0*/
> =A0 =A0 =A0 tmp =3D record_first.next;
> =A0 =A0 =A0 record_first.next =3D tmp->next;
> - =A0 =A0 =A0xfree (tmp);
> + =A0 =A0 =A0tmp->next->prev =3D &record_first;
> +
> + =A0 =A0 =A0/* tmp is now isolated, and can be deleted. =A0*/
> + =A0 =A0 =A0if (record_entry_release (tmp) =3D=3D record_end)
> + =A0 =A0 =A0 {
> + =A0 =A0 =A0 =A0 record_insn_num--;
> + =A0 =A0 =A0 =A0 break; =A0 =A0 =A0 =A0/* End loop at first record_end. =
=A0*/
> + =A0 =A0 =A0 }
>
> =A0 =A0 =A0 if (!record_first.next)
> =A0 =A0 =A0 =A0{
> =A0 =A0 =A0 =A0 =A0gdb_assert (record_insn_num =3D=3D 1);
> - =A0 =A0 =A0 =A0 break;
> + =A0 =A0 =A0 =A0 break; =A0 =A0 =A0 =A0/* End loop when list is empty. =
=A0*/
> =A0 =A0 =A0 =A0}
> -
> - =A0 =A0 =A0record_first.next->prev =3D &record_first;
> -
> - =A0 =A0 =A0if (type =3D=3D record_end)
> - =A0 =A0 =A0 break;
> =A0 =A0 }
> =A0}
>
> @@ -239,10 +329,10 @@ record_arch_list_add (struct record_entr
> =A0 =A0 }
> =A0}
>
> -/* Record the value of a register NUM to record_arch_list. =A0*/
> +/* Record the value of a register REGNUM to record_arch_list. =A0*/
>
> =A0int
> -record_arch_list_add_reg (struct regcache *regcache, int num)
> +record_arch_list_add_reg (struct regcache *regcache, int regnum)
> =A0{
> =A0 struct record_entry *rec;
>
> @@ -250,16 +340,11 @@ record_arch_list_add_reg (struct regcach
> =A0 =A0 fprintf_unfiltered (gdb_stdlog,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"Process record: add regis=
ter num =3D %d to "
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"record list.\n",
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 num);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 regnum);
>
> - =A0rec =3D (struct record_entry *) xmalloc (sizeof (struct record_entry=
));
> - =A0rec->u.reg.val =3D (gdb_byte *) xmalloc (MAX_REGISTER_SIZE);
> - =A0rec->prev =3D NULL;
> - =A0rec->next =3D NULL;
> - =A0rec->type =3D record_reg;
> - =A0rec->u.reg.num =3D num;
> + =A0rec =3D record_reg_alloc (regcache, regnum);
>
> - =A0regcache_raw_read (regcache, num, rec->u.reg.val);
> + =A0regcache_raw_read (regcache, regnum, rec->u.reg.val);
>
> =A0 record_arch_list_add (rec);
>
> @@ -280,17 +365,10 @@ record_arch_list_add_mem (CORE_ADDR addr
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"record list.\n",
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0paddress (target_gdbarch, =
addr), len);
>
> - =A0if (!addr)
> + =A0if (!addr) =A0 /* FIXME: Why? =A0Some arch must permit it... */
> =A0 =A0 return 0;
>
> - =A0rec =3D (struct record_entry *) xmalloc (sizeof (struct record_entry=
));
> - =A0rec->u.mem.val =3D (gdb_byte *) xmalloc (len);
> - =A0rec->prev =3D NULL;
> - =A0rec->next =3D NULL;
> - =A0rec->type =3D record_mem;
> - =A0rec->u.mem.addr =3D addr;
> - =A0rec->u.mem.len =3D len;
> - =A0rec->u.mem.mem_entry_not_accessible =3D 0;
> + =A0rec =3D record_mem_alloc (addr, len);
>
> =A0 if (target_read_memory (addr, rec->u.mem.val, len))
> =A0 =A0 {
> @@ -299,8 +377,7 @@ record_arch_list_add_mem (CORE_ADDR addr
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"Process record: e=
rror reading memory at "
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"addr =3D %s len =
=3D %d.\n",
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0paddress (target_g=
dbarch, addr), len);
> - =A0 =A0 =A0xfree (rec->u.mem.val);
> - =A0 =A0 =A0xfree (rec);
> + =A0 =A0 =A0record_mem_release (rec);
> =A0 =A0 =A0 return -1;
> =A0 =A0 }
>
> @@ -320,10 +397,7 @@ record_arch_list_add_end (void)
> =A0 =A0 fprintf_unfiltered (gdb_stdlog,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"Process record: add end t=
o arch list.\n");
>
> - =A0rec =3D (struct record_entry *) xmalloc (sizeof (struct record_entry=
));
> - =A0rec->prev =3D NULL;
> - =A0rec->next =3D NULL;
> - =A0rec->type =3D record_end;
> + =A0rec =3D record_end_alloc ();
> =A0 rec->u.end.sigval =3D TARGET_SIGNAL_0;
>
> =A0 record_arch_list_add (rec);
> @@ -818,7 +892,7 @@ record_wait (struct target_ops *ops,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0{
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (target_write_memory (recor=
d_list->u.mem.addr,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0record_list->u.mem.val,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0record_list->u.mem.val,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 record_list->u.mem.len))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0{
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (execution_directio=
n !=3D EXEC_REVERSE)
> @@ -1059,7 +1133,7 @@ record_store_registers (struct target_op
> =A0 =A0 =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0 =A0/* Destroy the record from here forward. =A0*/
> - =A0 =A0 =A0 =A0 record_list_release_next ();
> + =A0 =A0 =A0 =A0 record_list_release_following (record_list);
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 record_registers_change (regcache, regno);
> @@ -1091,7 +1165,7 @@ record_xfer_partial (struct target_ops *
> =A0 =A0 =A0 =A0 =A0 =A0error (_("Process record canceled the operation.")=
);
>
> =A0 =A0 =A0 =A0 =A0/* Destroy the record from here forward. =A0*/
> - =A0 =A0 =A0 =A0 record_list_release_next ();
> + =A0 =A0 =A0 =A0 record_list_release_following (record_list);
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 /* Check record_insn_num */
> @@ -1231,7 +1305,7 @@ cmd_record_delete (char *args, int from_
> =A0 =A0 =A0 =A0 =A0if (!from_tty || query (_("Delete the log from this po=
int forward "
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"a=
nd begin to record the running message
> "
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"a=
t current PC?")))
> - =A0 =A0 =A0 =A0 =A0 record_list_release_next ();
> + =A0 =A0 =A0 =A0 =A0 record_list_release_following (record_list);
> =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 else
> =A0 =A0 =A0 =A0 =A0printf_unfiltered (_("Already at end of record list.\n=
"));
>
>