On Fri, Jun 12, 2015 at 2:28 PM, Jeff King <peff@xxxxxxxx> wrote:
> We carefully check that our pkt buffer has enough characters
> before seeing if it starts with "PACK". The intent is to
> avoid reading random memory if we get a short buffer like
> "PAC".
>
> However, we know that the traced packets are always
> NUL-terminated. They come from one of these sources:
>
> 1. A string literal.
>
> 2. `format_packet`, which uses a strbuf.
>
> 3. `packet_read`, which defensively NUL-terminates what we
> read.
>
> We can therefore drop the length checks, as we know we will
> hit the trailing NUL if we have a short input.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> pkt-line.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/pkt-line.c b/pkt-line.c
> index 187a229..0477d2e 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -24,8 +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 ((len >= 4 && starts_with(buf, "PACK")) ||
> - (len >= 5 && starts_with(buf+1, "PACK"))) {
So I wondered why there is a possible extra character in front of PACK.
So I run the blame machinery and ended up with bbc30f9963 (add
packet tracing debug code, 2011-02-24), which was also authored
by you. Where does the extra char come from?
Would it be better for readability to write it as
int offset = 0;
if (*buf == CHARACTER_STEFAN_IS_WONDERING_ABOUT)
/* ignore char foo because bar */
offset++;
if (starts_with(buf+offset, "PACK") {
...
> + if (starts_with(buf, "PACK") || starts_with(buf + 1, "PACK")) {
> strbuf_addstr(&out, "PACK ...");
> trace_disable(&trace_packet);
> }
> --
> 2.4.2.752.geeb594a
>
--
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