This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Extend tsave: save start time, stop time, user and notes
- From: Pedro Alves <palves at redhat dot com>
- To: Dmitry Kozlov <dmitry_kozlov at mentor dot com>
- Cc: Yao Qi <yao at codesourcery dot com>, gdb-patches at sourceware dot org, "'Stan_Shebs at mentor dot com'" <Stan_Shebs at mentor dot com>, Vladimir Prus <vladimir at codesourcery dot com>
- Date: Thu, 11 Oct 2012 18:09:51 +0100
- Subject: Re: [PATCH] Extend tsave: save start time, stop time, user and notes
- References: <506C4719.3030900@mentor.com> <5072CB78.8030908@codesourcery.com> <5076EB66.6080307@mentor.com>
On 10/11/2012 04:53 PM, Dmitry Kozlov wrote:
>> Logically, this patch depends on your previous patch
>>
>> PATCH fix start-time and stop-time in trace-status
>> http://sourceware.org/ml/gdb-patches/2012-09/msg00621.html
>>
>> otherwise, we generate start-time and stop-time in trace file in decimal format, while gdb still interprets it as hex.
Since gdbserver and gdb have always been in disagreement (IOW, this
never worked correctly with gdbserver, so we're free to change it), and
the RSP uses hex everywhere (*), let's fix gdbserver instead.
@cindex remote protocol, field separator
Fields within the packet should be separated using @samp{,} @samp{;} or
@samp{:}. Except where otherwise noted all numbers are represented in
@sc{hex} with leading zeros suppressed.
> +2012-10-04 Dmitry Kozlov <ddk@codesourcery.com>
> +
> + * tracepoint.c (trace_save): Add saving starttime, stoptime, user and notes.
> +
> +2012-10-04 Dmitry Kozlov <ddk@mentor.com>
> +
> + * tracepoint.c (trace_status_command): Fix type of printf arg.
> + (trace_status_mi): Likewise.
Looks like something stale (two entries).
> +
> 2012-10-03 Doug Evans <dje@google.com>
>
> PR symtab/14601
> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
> index 0f94150..959ede5 100644
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -3018,10 +3018,11 @@ trace_save (const char *filename, int target_does_save)
> (ts->running ? '1' : '0'), stop_reason_names[ts->stop_reason]);
> if (ts->stop_reason == tracepoint_error)
> {
> - char *buf = (char *) alloca (strlen (ts->stop_desc) * 2 + 1);
> + char *buf = (char *) xmalloc (strlen (ts->stop_desc) * 2 + 1);
>
> bin2hex ((gdb_byte *) ts->stop_desc, buf, 0);
> fprintf (fp, ":%s", buf);
> + xfree (buf);
> }
> fprintf (fp, ":%x", ts->stopping_tracepoint);
> if (ts->traceframe_count >= 0)
> @@ -3036,6 +3037,35 @@ trace_save (const char *filename, int target_does_save)
> fprintf (fp, ";disconn:%x", ts->disconnected_tracing);
> if (ts->circular_buffer)
> fprintf (fp, ";circular:%x", ts->circular_buffer);
> + if (ts->start_time)
> + {
> + fprintf (fp, ";starttime:%ld%06ld",
> + (long int) (ts->start_time / 1000000),
> + (long int) (ts->start_time % 1000000));
See above. Numbers in the remote protocol are represented in hex.
> + }
> + if (ts->stop_time)
> + {
> + fprintf (fp, ";stoptime:%ld%06ld",
> + (long int) (ts->stop_time / 1000000),
> + (long int) (ts->stop_time % 1000000));
> + }
> + if (ts->notes && ts->notes[0] != 0 )
Spurious space after 0.
> + {
> + char *buf = (char *) xmalloc (strlen (ts->notes) * 2 + 1);
Unnecessary cast.
> +
> + bin2hex ((gdb_byte *) ts->notes, buf, 0);
> + fprintf (fp, ";notes:%s", buf);
> + xfree (buf);
Indentation is off (compared to ts->stop_time branch above).
Check tabs vs spaces.
> + }
> + if (ts->user_name)
As Yao pointed out, shouldn't this get the same check for empty string?
> + {
> + char *buf = (char *) xmalloc (strlen (ts->user_name) * 2 + 1);
> +
> + bin2hex ((gdb_byte *) ts->user_name, buf, 0);
> + fprintf (fp, ";username:%s", buf);
> + xfree (buf);
> + }
--
Pedro Alves