* Target-specific command logging facility
@ 2007-11-29 15:44 Maciej W. Rozycki
2007-11-29 18:20 ` Jim Blandy
0 siblings, 1 reply; 7+ messages in thread
From: Maciej W. Rozycki @ 2007-11-29 15:44 UTC (permalink / raw)
To: gdb-patches; +Cc: Maciej W. Rozycki
Hello,
In preparation for submitting support for the MDI target I propose the
following enhancement to the command logging facility. Right now
serial_log_command() is called unconditionally which does nothing if the
serial target subsystem is not being used.
I propose to change this behaviour and provide a (*to_log_command)()
member of the target vector so that the callee may be selected as
appropriate. The change below is meant to preserve the current semantics
-- all the currently supported targets that make use of the serial target
subsystem initialise the new member to serial_log_command().
Tested using the mipsisa32-sde-elf target, with the mips-sim-sde32/-EB
and mips-sim-sde32/-EL boards with no regressions.
2007-11-29 Maciej W. Rozycki <macro@mips.com>
* target.c (update_current_target): Inherit to_log_command.
* target.h (struct target_ops). Add to_log_command.
(target_log_command): New macro.
* top.c (execute_command): Call target_log_command() if available
rather than serial_log_command().
* monitor.c (init_base_monitor_ops): Initialize to_log_command.
* remote-m32r-sdi.c (init_m32r_ops): Likewise.
* remote-mips.c (_initialize_remote_mips): Likewise.
* remote.c (init_remote_ops): Likewise.
OK to apply?
Maciej
14616.diff
Index: binutils-quilt/src/gdb/target.h
===================================================================
--- binutils-quilt.orig/src/gdb/target.h 2007-04-16 13:04:50.000000000 +0100
+++ binutils-quilt/src/gdb/target.h 2007-04-16 13:06:27.000000000 +0100
@@ -402,6 +402,7 @@
int);
struct exception_event_record *(*to_get_current_exception_event) (void);
char *(*to_pid_to_exec_file) (int pid);
+ void (*to_log_command) (const char *);
enum strata to_stratum;
int to_has_all_memory;
int to_has_memory;
@@ -1201,6 +1202,10 @@
extern const struct target_desc *target_read_description (struct target_ops *);
+/* Command logging facility. */
+
+#define target_log_command current_target.to_log_command
+
/* Routines for maintenance of the target structures...
add_target: Add a target to the list of all possible targets.
Index: binutils-quilt/src/gdb/remote-m32r-sdi.c
===================================================================
--- binutils-quilt.orig/src/gdb/remote-m32r-sdi.c 2007-04-16 12:14:49.000000000 +0100
+++ binutils-quilt/src/gdb/remote-m32r-sdi.c 2007-04-16 13:06:27.000000000 +0100
@@ -1595,6 +1595,7 @@
m32r_ops.to_create_inferior = m32r_create_inferior;
m32r_ops.to_mourn_inferior = m32r_mourn_inferior;
m32r_ops.to_stop = m32r_stop;
+ m32r_ops.to_log_command = serial_log_command;
m32r_ops.to_stratum = process_stratum;
m32r_ops.to_has_all_memory = 1;
m32r_ops.to_has_memory = 1;
Index: binutils-quilt/src/gdb/remote-mips.c
===================================================================
--- binutils-quilt.orig/src/gdb/remote-mips.c 2007-04-16 13:04:50.000000000 +0100
+++ binutils-quilt/src/gdb/remote-mips.c 2007-04-16 13:06:27.000000000 +0100
@@ -3321,6 +3321,7 @@
mips_ops.to_load = mips_load;
mips_ops.to_create_inferior = mips_create_inferior;
mips_ops.to_mourn_inferior = mips_mourn_inferior;
+ mips_ops.to_log_command = serial_log_command;
mips_ops.to_stratum = process_stratum;
mips_ops.to_has_all_memory = 1;
mips_ops.to_has_memory = 1;
Index: binutils-quilt/src/gdb/monitor.c
===================================================================
--- binutils-quilt.orig/src/gdb/monitor.c 2007-04-16 12:14:49.000000000 +0100
+++ binutils-quilt/src/gdb/monitor.c 2007-04-16 13:06:27.000000000 +0100
@@ -2229,6 +2229,7 @@
monitor_ops.to_mourn_inferior = monitor_mourn_inferior;
monitor_ops.to_stop = monitor_stop;
monitor_ops.to_rcmd = monitor_rcmd;
+ monitor_ops.to_log_command = serial_log_command;
monitor_ops.to_stratum = process_stratum;
monitor_ops.to_has_all_memory = 1;
monitor_ops.to_has_memory = 1;
Index: binutils-quilt/src/gdb/top.c
===================================================================
--- binutils-quilt.orig/src/gdb/top.c 2007-04-16 12:14:49.000000000 +0100
+++ binutils-quilt/src/gdb/top.c 2007-04-16 13:06:27.000000000 +0100
@@ -387,7 +387,8 @@
if (p == NULL)
return;
- serial_log_command (p);
+ if (target_log_command)
+ target_log_command (p);
while (*p == ' ' || *p == '\t')
p++;
Index: binutils-quilt/src/gdb/remote.c
===================================================================
--- binutils-quilt.orig/src/gdb/remote.c 2007-04-16 13:04:49.000000000 +0100
+++ binutils-quilt/src/gdb/remote.c 2007-04-16 13:06:27.000000000 +0100
@@ -6153,6 +6153,7 @@
remote_ops.to_stop = remote_stop;
remote_ops.to_xfer_partial = remote_xfer_partial;
remote_ops.to_rcmd = remote_rcmd;
+ remote_ops.to_log_command = serial_log_command;
remote_ops.to_get_thread_local_address = remote_get_thread_local_address;
remote_ops.to_stratum = process_stratum;
remote_ops.to_has_all_memory = 1;
Index: binutils-quilt/src/gdb/target.c
===================================================================
--- binutils-quilt.orig/src/gdb/target.c 2007-04-16 13:04:51.000000000 +0100
+++ binutils-quilt/src/gdb/target.c 2007-04-16 13:06:27.000000000 +0100
@@ -451,6 +451,7 @@
INHERIT (to_enable_exception_callback, t);
INHERIT (to_get_current_exception_event, t);
INHERIT (to_pid_to_exec_file, t);
+ INHERIT (to_log_command, t);
INHERIT (to_stratum, t);
INHERIT (to_has_all_memory, t);
INHERIT (to_has_memory, t);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Target-specific command logging facility
2007-11-29 15:44 Target-specific command logging facility Maciej W. Rozycki
@ 2007-11-29 18:20 ` Jim Blandy
2007-11-30 13:55 ` Maciej W. Rozycki
0 siblings, 1 reply; 7+ messages in thread
From: Jim Blandy @ 2007-11-29 18:20 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: gdb-patches, Maciej W. Rozycki
"Maciej W. Rozycki" <macro at mips.com> writes:
> In preparation for submitting support for the MDI target I propose the
> following enhancement to the command logging facility. Right now
> serial_log_command() is called unconditionally which does nothing if the
> serial target subsystem is not being used.
>
> I propose to change this behaviour and provide a (*to_log_command)()
> member of the target vector so that the callee may be selected as
> appropriate. The change below is meant to preserve the current semantics
> -- all the currently supported targets that make use of the serial target
> subsystem initialise the new member to serial_log_command().
>
> Tested using the mipsisa32-sde-elf target, with the mips-sim-sde32/-EB
> and mips-sim-sde32/-EL boards with no regressions.
>
> 2007-11-29 Maciej W. Rozycki <macro@mips.com>
>
> * target.c (update_current_target): Inherit to_log_command.
> * target.h (struct target_ops). Add to_log_command.
> (target_log_command): New macro.
> * top.c (execute_command): Call target_log_command() if available
> rather than serial_log_command().
> * monitor.c (init_base_monitor_ops): Initialize to_log_command.
> * remote-m32r-sdi.c (init_m32r_ops): Likewise.
> * remote-mips.c (_initialize_remote_mips): Likewise.
> * remote.c (init_remote_ops): Likewise.
>
> OK to apply?
This looks good; I have some minor suggestions.
> 14616.diff
> Index: binutils-quilt/src/gdb/target.h
> ===================================================================
> --- binutils-quilt.orig/src/gdb/target.h 2007-04-16 13:04:50.000000000 +0100
> +++ binutils-quilt/src/gdb/target.h 2007-04-16 13:06:27.000000000 +0100
> @@ -402,6 +402,7 @@
> int);
> struct exception_event_record *(*to_get_current_exception_event) (void);
> char *(*to_pid_to_exec_file) (int pid);
> + void (*to_log_command) (const char *);
> enum strata to_stratum;
> int to_has_all_memory;
> int to_has_memory;
> @@ -1201,6 +1202,10 @@
>
> extern const struct target_desc *target_read_description (struct target_ops *);
>
> +/* Command logging facility. */
> +
> +#define target_log_command current_target.to_log_command
Could we have parens around the definition here?
> Index: binutils-quilt/src/gdb/top.c
> ===================================================================
> --- binutils-quilt.orig/src/gdb/top.c 2007-04-16 12:14:49.000000000 +0100
> +++ binutils-quilt/src/gdb/top.c 2007-04-16 13:06:27.000000000 +0100
> @@ -387,7 +387,8 @@
> if (p == NULL)
> return;
>
> - serial_log_command (p);
> + if (target_log_command)
> + target_log_command (p);
I think it's better style to have the conditional in the definition of
target_log_command itself, so that every caller doesn't have to test
whether current target happens to have such a function. I grant this
is the only caller, but people do look at existing code for precedent.
If you fix those, this is fine to commit.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Target-specific command logging facility
2007-11-29 18:20 ` Jim Blandy
@ 2007-11-30 13:55 ` Maciej W. Rozycki
2007-12-07 15:07 ` Maciej W. Rozycki
0 siblings, 1 reply; 7+ messages in thread
From: Maciej W. Rozycki @ 2007-11-30 13:55 UTC (permalink / raw)
To: Jim Blandy; +Cc: gdb-patches, Maciej W. Rozycki
On Thu, 29 Nov 2007, Jim Blandy wrote:
> This looks good; I have some minor suggestions.
Thanks for your review.
> I think it's better style to have the conditional in the definition of
> target_log_command itself, so that every caller doesn't have to test
> whether current target happens to have such a function. I grant this
> is the only caller, but people do look at existing code for precedent.
Well, I have no strong preference and your suggestion sounds reasonable
indeed.
So can I assume the change below is OK then? Tested using the
mipsisa32-sde-elf target, with the mips-sim-sde32/-EB and
mips-sim-sde32/-EL boards with no regressions.
2006-02-13 Maciej W. Rozycki <macro@mips.com>
* target.c (update_current_target): Inherit to_log_command.
* target.h (struct target_ops). Add to_log_command.
(target_log_command): New macro.
* top.c (execute_command): Call target_log_command() rather than
serial_log_command().
* monitor.c (init_base_monitor_ops): Initialize to_log_command.
* remote-m32r-sdi.c (init_m32r_ops): Likewise.
* remote-mips.c (_initialize_remote_mips): Likewise.
* remote.c (init_remote_ops): Likewise.
Maciej
14616.diff
Index: binutils-quilt/src/gdb/target.h
===================================================================
--- binutils-quilt.orig/src/gdb/target.h 2007-11-30 11:47:30.000000000 +0000
+++ binutils-quilt/src/gdb/target.h 2007-11-30 12:04:46.000000000 +0000
@@ -404,6 +404,7 @@
int);
struct exception_event_record *(*to_get_current_exception_event) (void);
char *(*to_pid_to_exec_file) (int pid);
+ void (*to_log_command) (const char *);
enum strata to_stratum;
int to_has_all_memory;
int to_has_memory;
@@ -1130,6 +1131,14 @@
extern const struct target_desc *target_read_description (struct target_ops *);
+/* Command logging facility. */
+
+#define target_log_command(p) \
+ do \
+ if (current_target.to_log_command) \
+ (*current_target.to_log_command) (p); \
+ while (0)
+
/* Routines for maintenance of the target structures...
add_target: Add a target to the list of all possible targets.
Index: binutils-quilt/src/gdb/remote-m32r-sdi.c
===================================================================
--- binutils-quilt.orig/src/gdb/remote-m32r-sdi.c 2007-11-30 11:47:30.000000000 +0000
+++ binutils-quilt/src/gdb/remote-m32r-sdi.c 2007-11-30 11:48:02.000000000 +0000
@@ -1599,6 +1599,7 @@
m32r_ops.to_create_inferior = m32r_create_inferior;
m32r_ops.to_mourn_inferior = m32r_mourn_inferior;
m32r_ops.to_stop = m32r_stop;
+ m32r_ops.to_log_command = serial_log_command;
m32r_ops.to_stratum = process_stratum;
m32r_ops.to_has_all_memory = 1;
m32r_ops.to_has_memory = 1;
Index: binutils-quilt/src/gdb/remote-mips.c
===================================================================
--- binutils-quilt.orig/src/gdb/remote-mips.c 2007-11-30 11:47:30.000000000 +0000
+++ binutils-quilt/src/gdb/remote-mips.c 2007-11-30 11:48:02.000000000 +0000
@@ -3333,6 +3333,7 @@
mips_ops.to_load = mips_load;
mips_ops.to_create_inferior = mips_create_inferior;
mips_ops.to_mourn_inferior = mips_mourn_inferior;
+ mips_ops.to_log_command = serial_log_command;
mips_ops.to_stratum = process_stratum;
mips_ops.to_has_all_memory = 1;
mips_ops.to_has_memory = 1;
Index: binutils-quilt/src/gdb/monitor.c
===================================================================
--- binutils-quilt.orig/src/gdb/monitor.c 2007-11-30 11:47:30.000000000 +0000
+++ binutils-quilt/src/gdb/monitor.c 2007-11-30 11:48:02.000000000 +0000
@@ -2233,6 +2233,7 @@
monitor_ops.to_mourn_inferior = monitor_mourn_inferior;
monitor_ops.to_stop = monitor_stop;
monitor_ops.to_rcmd = monitor_rcmd;
+ monitor_ops.to_log_command = serial_log_command;
monitor_ops.to_stratum = process_stratum;
monitor_ops.to_has_all_memory = 1;
monitor_ops.to_has_memory = 1;
Index: binutils-quilt/src/gdb/top.c
===================================================================
--- binutils-quilt.orig/src/gdb/top.c 2007-11-30 11:47:30.000000000 +0000
+++ binutils-quilt/src/gdb/top.c 2007-11-30 11:48:02.000000000 +0000
@@ -385,7 +385,7 @@
if (p == NULL)
return;
- serial_log_command (p);
+ target_log_command (p);
while (*p == ' ' || *p == '\t')
p++;
Index: binutils-quilt/src/gdb/remote.c
===================================================================
--- binutils-quilt.orig/src/gdb/remote.c 2007-11-30 11:47:30.000000000 +0000
+++ binutils-quilt/src/gdb/remote.c 2007-11-30 11:48:03.000000000 +0000
@@ -6315,6 +6315,7 @@
remote_ops.to_stop = remote_stop;
remote_ops.to_xfer_partial = remote_xfer_partial;
remote_ops.to_rcmd = remote_rcmd;
+ remote_ops.to_log_command = serial_log_command;
remote_ops.to_get_thread_local_address = remote_get_thread_local_address;
remote_ops.to_stratum = process_stratum;
remote_ops.to_has_all_memory = 1;
Index: binutils-quilt/src/gdb/target.c
===================================================================
--- binutils-quilt.orig/src/gdb/target.c 2007-11-30 11:47:30.000000000 +0000
+++ binutils-quilt/src/gdb/target.c 2007-11-30 11:48:03.000000000 +0000
@@ -448,6 +448,7 @@
INHERIT (to_enable_exception_callback, t);
INHERIT (to_get_current_exception_event, t);
INHERIT (to_pid_to_exec_file, t);
+ INHERIT (to_log_command, t);
INHERIT (to_stratum, t);
INHERIT (to_has_all_memory, t);
INHERIT (to_has_memory, t);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Target-specific command logging facility
2007-11-30 13:55 ` Maciej W. Rozycki
@ 2007-12-07 15:07 ` Maciej W. Rozycki
2007-12-07 15:36 ` Daniel Jacobowitz
0 siblings, 1 reply; 7+ messages in thread
From: Maciej W. Rozycki @ 2007-12-07 15:07 UTC (permalink / raw)
To: Jim Blandy; +Cc: gdb-patches, Maciej W. Rozycki
On Fri, 30 Nov 2007, Maciej W. Rozycki wrote:
> 2006-02-13 Maciej W. Rozycki <macro@mips.com>
>
> * target.c (update_current_target): Inherit to_log_command.
> * target.h (struct target_ops). Add to_log_command.
> (target_log_command): New macro.
> * top.c (execute_command): Call target_log_command() rather than
> serial_log_command().
> * monitor.c (init_base_monitor_ops): Initialize to_log_command.
> * remote-m32r-sdi.c (init_m32r_ops): Likewise.
> * remote-mips.c (_initialize_remote_mips): Likewise.
> * remote.c (init_remote_ops): Likewise.
Having seen no objections I have committed it now.
Maciej
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Target-specific command logging facility
2007-12-07 15:07 ` Maciej W. Rozycki
@ 2007-12-07 15:36 ` Daniel Jacobowitz
2007-12-07 15:45 ` Maciej W. Rozycki
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Jacobowitz @ 2007-12-07 15:36 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Jim Blandy, gdb-patches, Maciej W. Rozycki
On Fri, Dec 07, 2007 at 03:02:12PM +0000, Maciej W. Rozycki wrote:
> Having seen no objections I have committed it now.
Jim approved this, so it's fine - but could you explain the use case
for this? Why should it be target specific?
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Target-specific command logging facility
2007-12-07 15:36 ` Daniel Jacobowitz
@ 2007-12-07 15:45 ` Maciej W. Rozycki
2007-12-07 16:21 ` Daniel Jacobowitz
0 siblings, 1 reply; 7+ messages in thread
From: Maciej W. Rozycki @ 2007-12-07 15:45 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: Jim Blandy, gdb-patches, Maciej W. Rozycki
On Fri, 7 Dec 2007, Daniel Jacobowitz wrote:
> > Having seen no objections I have committed it now.
>
> Jim approved this, so it's fine - but could you explain the use case
> for this? Why should it be target specific?
MDI support provides its own logging facility which lets the user ask for
commands issued to GDB to be passed down to the target. The target may
than place these commands within its own log file (which will typically
provide additional data beyond the scope of GDB itself, such as the record
of transactions performed at the JTAG port) which lets one see what
actions at the target result from each of the GDB command. Useful for
debugging the MDI target itself.
Being self-contained I resisted from submitting this change together with
MDI not to obfuscate the view. I should be able to submit the MDI change
pretty soon now, though I have to take your suggestion on the handling of
threads first.
Maciej
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Target-specific command logging facility
2007-12-07 15:45 ` Maciej W. Rozycki
@ 2007-12-07 16:21 ` Daniel Jacobowitz
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Jacobowitz @ 2007-12-07 16:21 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Jim Blandy, gdb-patches, Maciej W. Rozycki
On Fri, Dec 07, 2007 at 03:36:05PM +0000, Maciej W. Rozycki wrote:
> MDI support provides its own logging facility which lets the user ask for
> commands issued to GDB to be passed down to the target. The target may
> than place these commands within its own log file (which will typically
> provide additional data beyond the scope of GDB itself, such as the record
> of transactions performed at the JTAG port) which lets one see what
> actions at the target result from each of the GDB command. Useful for
> debugging the MDI target itself.
Ah, I see. Yes, that does sound useful. Perhaps we should get MI
targets to log commands similarly, if they don't already, and a way to
do this same nice trick to stubs speaking the ordinary remote protocol.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2007-12-07 15:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-11-29 15:44 Target-specific command logging facility Maciej W. Rozycki
2007-11-29 18:20 ` Jim Blandy
2007-11-30 13:55 ` Maciej W. Rozycki
2007-12-07 15:07 ` Maciej W. Rozycki
2007-12-07 15:36 ` Daniel Jacobowitz
2007-12-07 15:45 ` Maciej W. Rozycki
2007-12-07 16:21 ` Daniel Jacobowitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox