Don't print too much if remote_debug is on
Commit Message
If we turn "remote debug" on and GDB does some vFile operations,
a lot of things will be printed in the screen, which makes
"remote debug" useless.
This patch changes the code that we don't print messages if
messages are too long, greater than 512. Instead, print
"Sending packet: $vFile:pread:5,3fff,e0d12#c4...Packet received: [16384 bytes omitted]".
gdb:
2016-11-29 Yao Qi <yao.qi@linaro.org>
* remote.c (REMOTE_DEBUG_MAX_CHAR): New macro.
(putpkt_binary): Print if content length is less than
REMOTE_DEBUG_MAX_CHAR.
(getpkt_or_notif_sane_1): Likewise.
---
gdb/remote.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
Comments
On 11/29/2016 09:38 AM, Yao Qi wrote:
> If we turn "remote debug" on and GDB does some vFile operations,
> a lot of things will be printed in the screen, which makes
> "remote debug" useless.
>
> This patch changes the code that we don't print messages if
> messages are too long, greater than 512. Instead, print
> "Sending packet: $vFile:pread:5,3fff,e0d12#c4...Packet received: [16384 bytes omitted]".
How about not printing binary data at all and instead print some useful
information about contents being sent? Like the number of bytes and
maybe how many still need to be read?
On Tue, Nov 29, 2016 at 09:45:59AM -0600, Luis Machado wrote:
> On 11/29/2016 09:38 AM, Yao Qi wrote:
> >If we turn "remote debug" on and GDB does some vFile operations,
> >a lot of things will be printed in the screen, which makes
> >"remote debug" useless.
> >
> >This patch changes the code that we don't print messages if
> >messages are too long, greater than 512. Instead, print
> >"Sending packet: $vFile:pread:5,3fff,e0d12#c4...Packet received: [16384 bytes omitted]".
>
> How about not printing binary data at all and instead print some
> useful information about contents being sent? Like the number of
> bytes and maybe how many still need to be read?
In putpkt/getpkt, we don't know the data is binary or not, unless we
pass an additional parameter to indicate this. Then, we need to
go through every calls to putpkt/getpkt, check the data is plain text
or binary, so pass the right value to putpkt/getpkt.
The binary and plain data is mixed in the buffer in some packets, like
"vFile:pwrite: fd, offset, data". If we want to print
"Sending packet: $vFile:pwrite:5,e0d12,[16384 bytes]#c4" in the debug
output, we need to move the debugging output from buffer level to
packet level. I agree it is better than
"Sending packet: [16384 bytes omitted]" which is what my patch does.
We can omit the received packet if it is more than REMOTE_DEBUG_MAX_CHAR
chars; if the sent packet is more than REMOTE_DEBUG_MAX_CHAR chars, only
print the first 50 chars, and omit the rest of them, so the debug
output is like,
Sending packet: $vFile:pread:5,3fff,e0d12#c4...Packet received: [16384 bytes omitted]
Sending packet: $vFile:pwrite:5,e0d12,xxxyyyzzz[384 bytes omitted] ... Packet received: 358
What do you think?
On 11/30/2016 06:54 AM, Yao Qi wrote:
> On Tue, Nov 29, 2016 at 09:45:59AM -0600, Luis Machado wrote:
>> On 11/29/2016 09:38 AM, Yao Qi wrote:
>>> If we turn "remote debug" on and GDB does some vFile operations,
>>> a lot of things will be printed in the screen, which makes
>>> "remote debug" useless.
>>>
>>> This patch changes the code that we don't print messages if
>>> messages are too long, greater than 512. Instead, print
>>> "Sending packet: $vFile:pread:5,3fff,e0d12#c4...Packet received: [16384 bytes omitted]".
>>
>> How about not printing binary data at all and instead print some
>> useful information about contents being sent? Like the number of
>> bytes and maybe how many still need to be read?
>
> In putpkt/getpkt, we don't know the data is binary or not, unless we
> pass an additional parameter to indicate this. Then, we need to
> go through every calls to putpkt/getpkt, check the data is plain text
> or binary, so pass the right value to putpkt/getpkt.
Ah, that's unfortunate.
>
> The binary and plain data is mixed in the buffer in some packets, like
> "vFile:pwrite: fd, offset, data". If we want to print
> "Sending packet: $vFile:pwrite:5,e0d12,[16384 bytes]#c4" in the debug
> output, we need to move the debugging output from buffer level to
> packet level. I agree it is better than
> "Sending packet: [16384 bytes omitted]" which is what my patch does.
>
> We can omit the received packet if it is more than REMOTE_DEBUG_MAX_CHAR
> chars; if the sent packet is more than REMOTE_DEBUG_MAX_CHAR chars, only
> print the first 50 chars, and omit the rest of them, so the debug
> output is like,
>
> Sending packet: $vFile:pread:5,3fff,e0d12#c4...Packet received: [16384 bytes omitted]
> Sending packet: $vFile:pwrite:5,e0d12,xxxyyyzzz[384 bytes omitted] ... Packet received: 358
>
> What do you think?
>
I think it is an improvement nonetheless. Personally i still find
particular lengthy replies useful, like the XML descriptions. But all
the binary data is too distracting, hence why i was suggesting only
binary streams being restricted.
I'm fine with your version.
On 11/30/2016 12:54 PM, Yao Qi wrote:
> On Tue, Nov 29, 2016 at 09:45:59AM -0600, Luis Machado wrote:
>> On 11/29/2016 09:38 AM, Yao Qi wrote:
>>> If we turn "remote debug" on and GDB does some vFile operations,
>>> a lot of things will be printed in the screen, which makes
>>> "remote debug" useless.
>>>
>>> This patch changes the code that we don't print messages if
>>> messages are too long, greater than 512. Instead, print
>>> "Sending packet: $vFile:pread:5,3fff,e0d12#c4...Packet received: [16384 bytes omitted]".
>>
>> How about not printing binary data at all and instead print some
>> useful information about contents being sent? Like the number of
>> bytes and maybe how many still need to be read?
>
> In putpkt/getpkt, we don't know the data is binary or not, unless we
> pass an additional parameter to indicate this. Then, we need to
> go through every calls to putpkt/getpkt, check the data is plain text
> or binary, so pass the right value to putpkt/getpkt.
>
> The binary and plain data is mixed in the buffer in some packets, like
> "vFile:pwrite: fd, offset, data". If we want to print
> "Sending packet: $vFile:pwrite:5,e0d12,[16384 bytes]#c4" in the debug
> output, we need to move the debugging output from buffer level to
> packet level. I agree it is better than
> "Sending packet: [16384 bytes omitted]" which is what my patch does.
>
> We can omit the received packet if it is more than REMOTE_DEBUG_MAX_CHAR
> chars; if the sent packet is more than REMOTE_DEBUG_MAX_CHAR chars, only
> print the first 50 chars, and omit the rest of them, so the debug
> output is like,
>
> Sending packet: $vFile:pread:5,3fff,e0d12#c4...Packet received: [16384 bytes omitted]
> Sending packet: $vFile:pwrite:5,e0d12,xxxyyyzzz[384 bytes omitted] ... Packet received: 358
>
> What do you think?
I was just now writing a reply to the OP to suggest something like
that, not noticing this reply. So +1 from me.
But I'm not sure I follow the "50" in "50 chars".
Why not instead:
If REMOTE_DEBUG_MAX_CHAR is more than REMOTE_DEBUG_MAX_CHAR chars,
only print the first REMOTE_DEBUG_MAX_CHAR chars.
Thanks,
Pedro Alves
Luis Machado wrote:
> On 11/30/2016 06:54 AM, Yao Qi wrote:
> > The binary and plain data is mixed in the buffer in some packets, like
> > "vFile:pwrite: fd, offset, data". If we want to print
> > "Sending packet: $vFile:pwrite:5,e0d12,[16384 bytes]#c4" in the debug
> > output, we need to move the debugging output from buffer level to
> > packet level. I agree it is better than
> > "Sending packet: [16384 bytes omitted]" which is what my patch does.
> >
> > We can omit the received packet if it is more than REMOTE_DEBUG_MAX_CHAR
> > chars; if the sent packet is more than REMOTE_DEBUG_MAX_CHAR chars, only
> > print the first 50 chars, and omit the rest of them, so the debug
> > output is like,
> >
> > Sending packet: $vFile:pread:5,3fff,e0d12#c4...Packet received: [16384 bytes omitted]
> > Sending packet: $vFile:pwrite:5,e0d12,xxxyyyzzz[384 bytes omitted] ... Packet received: 358
> >
> > What do you think?
>
> I think it is an improvement nonetheless. Personally i still find
> particular lengthy replies useful, like the XML descriptions. But
> all the binary data is too distracting, hence why i was suggesting
> only binary streams being restricted.
>
> I'm fine with your version.
I haven't checked, but it might be trivial to spot and not strip XML
by looking for "<?xml" in the first few bytes if the packet.
However this happens, removing the huge binary packets gets my +1.
(I know I'm responsible for a large increase in those recently,
sorry!)
Cheers,
Gary
@@ -283,6 +283,11 @@ typedef unsigned char threadref[OPAQUETHREADBYTES];
#define MAXTHREADLISTRESULTS 32
+/* The max number of chars in debug output. Output more than this number
+ of chars is omitted. */
+
+#define REMOTE_DEBUG_MAX_CHAR 512
+
/* Data for the vFile:pread readahead cache. */
struct readahead_cache
@@ -8749,9 +8754,16 @@ putpkt_binary (const char *buf, int cnt)
{
*p = '\0';
- std::string str = escape_buffer (buf2, p - buf2);
+ fprintf_unfiltered (gdb_stdlog, "Sending packet: ");
+ ptrdiff_t len = p - buf2;
+ if (len <= REMOTE_DEBUG_MAX_CHAR)
+ {
+ std::string str = escape_buffer (buf2, len);
- fprintf_unfiltered (gdb_stdlog, "Sending packet: %s...", str.c_str ());
+ fprintf_unfiltered (gdb_stdlog, "%s...", str.c_str ());
+ }
+ else
+ fprintf_unfiltered (gdb_stdlog, "[%td bytes omitted]...", len);
gdb_flush (gdb_stdlog);
}
remote_serial_write (buf2, p - buf2);
@@ -9179,9 +9191,15 @@ getpkt_or_notif_sane_1 (char **buf, long *sizeof_buf, int forever,
{
if (remote_debug)
{
- std::string str = escape_buffer (*buf, val);
+ fprintf_unfiltered (gdb_stdlog, "Packet received: ");
+ if (val <= REMOTE_DEBUG_MAX_CHAR)
+ {
+ std::string str = escape_buffer (*buf, val);
- fprintf_unfiltered (gdb_stdlog, "Packet received: %s\n", str.c_str ());
+ fprintf_unfiltered (gdb_stdlog, "%s\n", str.c_str ());
+ }
+ else
+ fprintf_unfiltered (gdb_stdlog, "[%d bytes omitted]\n", val);
}
/* Skip the ack char if we're in no-ack mode. */