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.
[...]
> --- 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?
> +++ 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.)
[...]
> @@ -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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html