From: "Pedro Alves (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: Luis Machado <luis.machado@linaro.org>, gdb-patches@sourceware.org
Cc: Tom Tromey <tromey@sourceware.org>
Subject: [review v3] [Debugging output] Make remote packet truncation length adjustable
Date: Thu, 21 Nov 2019 16:31:00 -0000 [thread overview]
Message-ID: <20191121163121.5790A2816F@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1574263562000.I2e871b37bfcaa6376537c3fe3db8f016dd806a7c@gnutoolchain-gerrit.osci.io>
Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/691
......................................................................
Patch Set 3:
> > > I'm not too sure packet-length-limit is good. Suggestions are welcome.
I agree "set remote packet-length-limit" isn't good, since I'd expect a setting by that name to control the actual max size of gdb's packet buffer.
> >
> > I tend to think something in the "set debug" namespace would be better,
> > since it's a setting related to "set debug remote".
>
> I contemplated that, but upon looking at what sorts of options were available via "set debug", they were related to producing debugging output only, not adjusting how the debugging output was produced. So i went for more locality by putting it into "set remote".
>
> To be honest, i don't think it fits in any of those two. But i'm okay with going for "set debug remote-log-length" based on feedback.
>
> >
> > How about "set debug remote-log-length"?
> >
> > This patch also needs a documentation change and a NEWS entry.
>
> I'll put something together.
I'm not thrilled with "remote-log-length" because "length" alone doesn't convey "max", or "limit".
Your original "packet-length-limit" did, but it was under the wrong namespace.
Including "log" in the setting name seems redundant, since "set debug" settings all control logging in some form.
"packet-length-limit" wasn't 100% correct, since it's not really the size of the packet that counts, it's how much you'd display, including escaping, but that seems like a minor issue.
Let's look at how the option is described:
"Sets the number of characters to display for each remote packet"
Note that it doesn't talk about "log" and doesn't say "length".
If you start from that description, then the obvious setting name would be around:
set debug remote-max-chars
set debug remote-packet-max-characters
set debug remote-packet-max-chars
set debug remote-max-packet-chars
I think my preferred one would be "remote-packet-max-chars", since the limit only applies to packet logging.
Which is not unlike the existing macro name:
/* The max number of chars in debug output. The rest of chars are
omitted. */
#define REMOTE_DEBUG_MAX_CHAR 512
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I2e871b37bfcaa6376537c3fe3db8f016dd806a7c
Gerrit-Change-Number: 691
Gerrit-PatchSet: 3
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Thu, 21 Nov 2019 16:31:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
next prev parent reply other threads:[~2019-11-21 16:31 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-20 15:26 [review] " Luis Machado (Code Review)
2019-11-20 15:27 ` Luis Machado (Code Review)
2019-11-20 18:12 ` Tom Tromey (Code Review)
2019-11-20 18:42 ` Luis Machado (Code Review)
2019-11-20 20:35 ` [review v2] " Luis Machado (Code Review)
2019-11-21 15:05 ` Eli Zaretskii
2019-11-21 14:41 ` Tom Tromey (Code Review)
2019-11-21 15:17 ` [review v3] " Luis Machado (Code Review)
2019-11-21 15:25 ` Eli Zaretskii
2019-11-21 16:31 ` Pedro Alves (Code Review) [this message]
2019-11-21 17:15 ` [review v4] " Luis Machado (Code Review)
2019-11-21 18:55 ` Pedro Alves (Code Review)
2019-11-21 21:49 ` Luis Machado (Code Review)
2019-11-21 21:49 ` Luis Machado (Code Review)
2019-11-21 22:03 ` [review v5] " Luis Machado (Code Review)
2019-11-22 7:34 ` Eli Zaretskii
2019-11-22 15:22 ` Pedro Alves (Code Review)
2019-11-25 13:31 ` [review v6] " Luis Machado (Code Review)
2019-11-25 15:34 ` Eli Zaretskii
2019-11-25 13:33 ` Luis Machado (Code Review)
2019-11-25 15:30 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-11-25 15:30 ` Sourceware to Gerrit sync (Code Review)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191121163121.5790A2816F@gnutoolchain-gerrit.osci.io \
--to=gerrit@gnutoolchain-gerrit.osci.io \
--cc=gdb-patches@sourceware.org \
--cc=gnutoolchain-gerrit@osci.io \
--cc=luis.machado@linaro.org \
--cc=tromey@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox