From: Stefan Beller <sbeller@google.com>
Date: Fri, 12 Jun 2015 21:39:01 GMT
Subject: Re: [PATCH 2/3] pkt-line: tighten sideband PACK check when tracing
Message-ID: <CAGZ79kb+DBUqAy45+Bc=MTA6HuGrj9MzyODZGTvOcP_XefAu=A@mail.gmail.com>
In-Reply-To: <20150612212814.GB25757@peff.net>

On Fri, Jun 12, 2015 at 2:28 PM, Jeff King <peff@peff.net> wrote:
> To find the start of the pack data, we accept the word PACK
> at the beginning of any sideband channel, even though what
> we really want is to find the pack data on channel 1. In
> practice this doesn't matter, as sideband-2 messages tend to
> start with "error:" or similar, but it is a good idea to be
> explicit (especially as we add more code in this area, we
> will rely on this assumption).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  pkt-line.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/pkt-line.c b/pkt-line.c
> index 0477d2e..e75af02 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -24,7 +24,7 @@ static void packet_trace(const char *buf, unsigned int len, int write)
>         strbuf_addf(&out, "packet: %12s%c ",
>                     packet_trace_prefix, write ? '>' : '<');
>
> -       if (starts_with(buf, "PACK") || starts_with(buf + 1, "PACK")) {
> +       if (starts_with(buf, "PACK") || starts_with(buf, "\1PACK")) {

This answers the question on the previous patch actually, maybe the
code could be improved to

    if (is_sidechannel(out, ...)
        out++;
    if (starts_with(buf, "PACK") {
        ...


>                 strbuf_addstr(&out, "PACK ...");
>                 trace_disable(&trace_packet);
>         }
> --
> 2.4.2.752.geeb594a
>