[patch] remote_set_trace_notes

[patch] remote_set_trace_notes

[Abid, Hafiz]

[-- Attachment #1: Type: text/plain, Size: 396 bytes --]


Hi All,
During review of trace-buffer-size patch, I was asked to move a change  
into a separate patch. This is simple change in remote_set_trace_notes  
that will make this function return if all arguments are NULL.

2012-03-01  Stan Shebs  <stan@codesourcery.com>
	    Hafiz Abid Qadeer  <abidh@codesourcery.com>

	gdb/
	* remote.c (remote_set_trace_notes): Handle no-op case better.

[-- Attachment #2: set-trace-notes.patch --]
[-- Type: text/x-patch, Size: 1171 bytes --]

diff --git a/gdb/remote.c b/gdb/remote.c
index 88a57c8..8326ce8 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11050,6 +11050,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
   char *buf = rs->buf;
   char *endbuf = rs->buf + get_remote_packet_size ();
   int nbytes;
+  int any = 0;
 
   buf += xsnprintf (buf, endbuf - buf, "QTNotes:");
   if (user)
@@ -11058,6 +11059,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
       nbytes = bin2hex (user, buf, 0);
       buf += 2 * nbytes;
       *buf++ = ';';
+      any = 1;
     }
   if (notes)
     {
@@ -11065,6 +11067,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
       nbytes = bin2hex (notes, buf, 0);
       buf += 2 * nbytes;
       *buf++ = ';';
+      any = 1;
     }
   if (stop_notes)
     {
@@ -11072,7 +11075,11 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
       nbytes = bin2hex (stop_notes, buf, 0);
       buf += 2 * nbytes;
       *buf++ = ';';
+      any = 1;
     }
+  if (any == 0)
+    return 0;
+
   /* Ensure the buffer is terminated.  */
   *buf = '\0';
 

Re: [patch] remote_set_trace_notes

[Pedro Alves]

On 03/01/2013 05:43 PM, Abid, Hafiz wrote:
> 
> Hi All,
> During review of trace-buffer-size patch, I was asked to move a change into a separate patch. This is simple change in remote_set_trace_notes that will make this function return if all arguments are NULL.

> 
> 2012-03-01  Stan Shebs  <stan@codesourcery.com>
>         Hafiz Abid Qadeer  <abidh@codesourcery.com>
> 
>     gdb/
>     * remote.c (remote_set_trace_notes): Handle no-op case better.

Should this really be a no-op?  I'm not sure this is correct.

Say you connect to a target that had been tracing
before (disconnected tracing), and had its trace
notes/user/etc. set.

GDB connects, the user sets up a new trace session,
but doesn't set notes/user/etc.  IOW, the (trace_user/trace_notes/etc.)
GDB variables are left with their NULL defaults.

"show trace-notes" shows:

  (gdb) show trace-notes
  The notes string to use for current and future trace runs is "".

The user starts the trace session with tstart.

With the patch, GDB skips sending "QTNotes:...".

The target reuses the user/notes from the previous session.

That can't be right.

Do the same experiment again, but do (literally)
"set trace-notes" before tstart.  This sets the trace_notes
variable to "" instead of NULL.  This time, "tstart"
actually clears the previous run's trace notes on the
target.

Seems to me there's actually a bug here, and NULL and ""
should be treated as equivalent by remote_set_trace_notes.

-- 
Pedro Alves