Re: [PATCH] patch-id.c: use strbuf instead of a fixed buffer

From: Jeff King <peff@peff.net>
Date: Sep 22, 2011, 17:17 UTC
Message-ID: <20110922171706.GB2934@sigill.intra.peff.net>
In-Reply-To: <4E79DBAE.5090505@elegosoft.com>

On Wed, Sep 21, 2011 at 02:42:22PM +0200, Michael Schubert wrote:

Show 8 quoted lines

> get_one_patchid() uses a rather dumb heuristic to determine if the
> passed buffer is part of the next commit. Whenever the first 40 bytes
> are a valid hexadecimal sha1 representation, get_one_patchid() returns
> next_sha1.
> Once the current line is longer than the fixed buffer, this will break
> (provided the additional bytes make a valid hexadecimal sha1). As a result
> patch-id returns incorrect results. Instead, user strbuf and read one
> line at a time.

A minor nit, but I think this is probably broken even if the additional bytes don't look like a valid sha1. It would look like cruft after the diff (since the lien doesn't start with plus, minus, or space), which means it would not be stirred into the sha1 mix.

>  builtin/patch-id.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)

The patch itself looks good to me, though. Thanks.

-Peff