This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Structure and simplify record log in record.c
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 Michael Snyder <msnyder@vmware.com>
* record.c (record_reg_alloc): New function.
(record_reg_release): New function.
(record_mem_alloc): New function.
(record_mem_release): New function.
(record_end_alloc): New function.
(record_end_release): New function.
(record_entry_release): New function.
(record_list_release): Simplify, call record_entry_release.
(record_list_release_next): Rename to record_list_release_following.
Simplify and call record_entry_release.
(record_list_release_first): Simplify, comment, and use
record_entry_release.
(record_arch_list_add_reg): Simplify, call record_reg_alloc.
(record_arch_list_add_mem): Simplify, call record_mem_alloc.
(record_arch_list_add_end): Simplify, call record_end_alloc.
Index: record.c
===================================================================
RCS file: /cvs/src/src/gdb/record.c,v
retrieving revision 1.21
diff -u -p -r1.21 record.c
--- record.c 15 Oct 2009 16:57:36 -0000 1.21
+++ record.c 15 Oct 2009 17:14:30 -0000
@@ -129,50 +129,143 @@ static int (*record_beneath_to_insert_br
static int (*record_beneath_to_remove_breakpoint) (struct gdbarch *,
struct bp_target_info *);
+/* Alloc and free functions for record_reg, record_mem, and record_end
+ entries. */
+
+/* Alloc a record_reg record entry. */
+
+static inline struct record_entry *
+record_reg_alloc (struct regcache *regcache, int regnum)
+{
+ struct record_entry *rec;
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+ rec = (struct record_entry *) xcalloc (1, sizeof (struct record_entry));
+ rec->type = record_reg;
+ rec->u.reg.num = regnum;
+ rec->u.reg.val = (gdb_byte *) xmalloc (MAX_REGISTER_SIZE);
+
+ return rec;
+}
+
+/* Free a record_reg record entry. */
+
+static inline void
+record_reg_release (struct record_entry *rec)
+{
+ gdb_assert (rec->type == record_reg);
+ xfree (rec->u.reg.val);
+ xfree (rec);
+}
+
+/* Alloc a record_mem record entry. */
+
+static inline struct record_entry *
+record_mem_alloc (CORE_ADDR addr, int len)
+{
+ struct record_entry *rec;
+
+ rec = (struct record_entry *) xcalloc (1, sizeof (struct record_entry));
+ rec->type = record_mem;
+ rec->u.mem.addr = addr;
+ rec->u.mem.len = len;
+ rec->u.mem.val = (gdb_byte *) xmalloc (len);
+
+ return rec;
+}
+
+/* Free a record_mem record entry. */
+
+static inline void
+record_mem_release (struct record_entry *rec)
+{
+ gdb_assert (rec->type == record_mem);
+ xfree (rec->u.mem.val);
+ xfree (rec);
+}
+
+/* Alloc a record_end record entry. */
+
+static inline struct record_entry *
+record_end_alloc (void)
+{
+ struct record_entry *rec;
+
+ rec = (struct record_entry *) xcalloc (1, sizeof (struct record_entry));
+ rec->type = record_end;
+
+ return rec;
+}
+
+/* Free a record_end record entry. */
+
+static inline void
+record_end_release (struct record_entry *rec)
+{
+ xfree (rec);
+}
+
+/* Free one record entry, any type.
+ Return entry->type, in case caller wants to know. */
+
+static inline enum record_type
+record_entry_release (struct record_entry *rec)
+{
+ enum record_type type = rec->type;
+
+ switch (type) {
+ case record_reg:
+ record_reg_release (rec);
+ break;
+ case record_mem:
+ record_mem_release (rec);
+ break;
+ case record_end:
+ record_end_release (rec);
+ break;
+ }
+ return type;
+}
+
+/* Free all record entries in list pointed to by REC. */
+
static void
record_list_release (struct record_entry *rec)
{
- struct record_entry *tmp;
-
if (!rec)
return;
while (rec->next)
- {
- rec = rec->next;
- }
+ rec = rec->next;
while (rec->prev)
{
- tmp = rec;
rec = rec->prev;
- if (tmp->type == record_reg)
- xfree (tmp->u.reg.val);
- else if (tmp->type == record_mem)
- xfree (tmp->u.mem.val);
- xfree (tmp);
+ record_entry_release (rec->next);
}
- if (rec != &record_first)
- xfree (rec);
+ if (rec == &record_first)
+ {
+ record_insn_num = 0;
+ record_first.next = NULL;
+ }
+ else
+ record_entry_release (rec);
}
+/* Free all record entries forward of the given list position. */
+
static void
-record_list_release_next (void)
+record_list_release_following (struct record_entry *rec)
{
- struct record_entry *rec = record_list;
struct record_entry *tmp = rec->next;
+
rec->next = NULL;
while (tmp)
{
rec = tmp->next;
- if (tmp->type == record_end)
+ if (record_entry_release (tmp) == record_end)
record_insn_num--;
- else if (tmp->type == record_reg)
- xfree (tmp->u.reg.val);
- else if (tmp->type == record_mem)
- xfree (tmp->u.mem.val);
- xfree (tmp);
tmp = rec;
}
}
@@ -185,34 +278,31 @@ record_list_release_next (void)
static void
record_list_release_first (void)
{
- struct record_entry *tmp = NULL;
- enum record_type type;
+ struct record_entry *tmp;
if (!record_first.next)
return;
+ /* Loop until a record_end. */
while (1)
{
- type = record_first.next->type;
-
- if (type == record_reg)
- xfree (record_first.next->u.reg.val);
- else if (type == record_mem)
- xfree (record_first.next->u.mem.val);
+ /* Cut record_first.next out of the linked list. */
tmp = record_first.next;
record_first.next = tmp->next;
- xfree (tmp);
+ tmp->next->prev = &record_first;
+
+ /* tmp is now isolated, and can be deleted. */
+ if (record_entry_release (tmp) == record_end)
+ {
+ record_insn_num--;
+ break; /* End loop at first record_end. */
+ }
if (!record_first.next)
{
gdb_assert (record_insn_num == 1);
- break;
+ break; /* End loop when list is empty. */
}
-
- record_first.next->prev = &record_first;
-
- if (type == record_end)
- break;
}
}
@@ -239,10 +329,10 @@ record_arch_list_add (struct record_entr
}
}
-/* Record the value of a register NUM to record_arch_list. */
+/* Record the value of a register REGNUM to record_arch_list. */
int
-record_arch_list_add_reg (struct regcache *regcache, int num)
+record_arch_list_add_reg (struct regcache *regcache, int regnum)
{
struct record_entry *rec;
@@ -250,16 +340,11 @@ record_arch_list_add_reg (struct regcach
fprintf_unfiltered (gdb_stdlog,
"Process record: add register num = %d to "
"record list.\n",
- num);
+ regnum);
- rec = (struct record_entry *) xmalloc (sizeof (struct record_entry));
- rec->u.reg.val = (gdb_byte *) xmalloc (MAX_REGISTER_SIZE);
- rec->prev = NULL;
- rec->next = NULL;
- rec->type = record_reg;
- rec->u.reg.num = num;
+ rec = record_reg_alloc (regcache, regnum);
- regcache_raw_read (regcache, num, rec->u.reg.val);
+ regcache_raw_read (regcache, regnum, rec->u.reg.val);
record_arch_list_add (rec);
@@ -280,17 +365,10 @@ record_arch_list_add_mem (CORE_ADDR addr
"record list.\n",
paddress (target_gdbarch, addr), len);
- if (!addr)
+ if (!addr) /* FIXME: Why? Some arch must permit it... */
return 0;
- rec = (struct record_entry *) xmalloc (sizeof (struct record_entry));
- rec->u.mem.val = (gdb_byte *) xmalloc (len);
- rec->prev = NULL;
- rec->next = NULL;
- rec->type = record_mem;
- rec->u.mem.addr = addr;
- rec->u.mem.len = len;
- rec->u.mem.mem_entry_not_accessible = 0;
+ rec = record_mem_alloc (addr, len);
if (target_read_memory (addr, rec->u.mem.val, len))
{
@@ -299,8 +377,7 @@ record_arch_list_add_mem (CORE_ADDR addr
"Process record: error reading memory at "
"addr = %s len = %d.\n",
paddress (target_gdbarch, addr), len);
- xfree (rec->u.mem.val);
- xfree (rec);
+ record_mem_release (rec);
return -1;
}
@@ -320,10 +397,7 @@ record_arch_list_add_end (void)
fprintf_unfiltered (gdb_stdlog,
"Process record: add end to arch list.\n");
- rec = (struct record_entry *) xmalloc (sizeof (struct record_entry));
- rec->prev = NULL;
- rec->next = NULL;
- rec->type = record_end;
+ rec = record_end_alloc ();
rec->u.end.sigval = TARGET_SIGNAL_0;
record_arch_list_add (rec);
@@ -818,7 +892,7 @@ record_wait (struct target_ops *ops,
else
{
if (target_write_memory (record_list->u.mem.addr,
- record_list->u.mem.val,
+ record_list->u.mem.val,
record_list->u.mem.len))
{
if (execution_direction != EXEC_REVERSE)
@@ -1059,7 +1133,7 @@ record_store_registers (struct target_op
}
/* Destroy the record from here forward. */
- record_list_release_next ();
+ record_list_release_following (record_list);
}
record_registers_change (regcache, regno);
@@ -1091,7 +1165,7 @@ record_xfer_partial (struct target_ops *
error (_("Process record canceled the operation."));
/* Destroy the record from here forward. */
- record_list_release_next ();
+ record_list_release_following (record_list);
}
/* Check record_insn_num */
@@ -1231,7 +1305,7 @@ cmd_record_delete (char *args, int from_
if (!from_tty || query (_("Delete the log from this point forward "
"and begin to record the running message "
"at current PC?")))
- record_list_release_next ();
+ record_list_release_following (record_list);
}
else
printf_unfiltered (_("Already at end of record list.\n"));