Re: [PATCH 7/8] add packet tracing debug code
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:50:39
Jeff King wrote:
This shows a trace of all packets coming in or out of a given program. This can help with debugging object negotiation or other protocol issues.
Sounds handy.
Packet tracing can be enabled with GIT_TRACE_PACKET=<foo>, where <foo> takes the same arguments as GIT_TRACE.
[...]
quoted hunk ↗ jump to hunk
--- a/builtin/clone.c +++ b/builtin/clone.c@@ -383,6 +383,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) junk_pid = getpid(); + packet_trace_identity("clone");
My first thought on reading this was "identity operation" (= pass-through), which left me a little confused. Maybe "packet_trace_set_identity" (or ...set_prefix) would be a little clearer?
quoted hunk ↗ jump to hunk
+++ b/pkt-line.c@@ -1,6 +1,51 @@
[...]
+ if ((len >= 4 && !prefixcmp(buf, "PACK")) ||
+ (len >= 5 && !prefixcmp(buf+1, "PACK"))) {
+ strbuf_addstr(&out, "PACK ...");
+ unsetenv(trace_key);
+ }
+ else {Style: should be cuddled, I think.
+ /* XXX we should really handle printable utf8 */
+ for (i = 0; i < len; i++) {Maybe using is_utf8, extended to take a length argument? (Assuming it's okay if non-ASCII is escaped in packets containing non-utf8 parts.) [...]
quoted hunk ↗ jump to hunk
@@ -39,11 +84,13 @@ ssize_t safe_write(int fd, const void *buf, ssize_t n) */ void packet_flush(int fd) { + packet_trace("0000", 4, 1); safe_write(fd, "0000", 4);
... I wonder if it would make sense to do
static void packet_writebuf(int fd, const char *buf, size_t buflen)
{
packet_trace(buf, buflen, 1);
safe_write(fd, buf, buflen);
}
Nah, that would include the size noise for even nonempty packets,
which your implementation conveniently omits.
Thanks, looks useful.