Re: [PATCH] general style: replaces memcmp() with proper starts_with()

From: Jeff King <peff@peff.net>
Date: Mar 13, 2014, 17:55 UTC
Message-ID: <20140313175515.GA18897@sigill.intra.peff.net>
In-Reply-To: <xmqq4n32j8i7.fsf@gitster.dls.corp.google.com>

On Thu, Mar 13, 2014 at 10:47:28AM -0700, Junio C Hamano wrote:

Show 17 quoted lines

> >> --- a/builtin/cat-file.c
> >> +++ b/builtin/cat-file.c
> >> @@ -82,7 +82,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
> >>  				enum object_type type;
> >>  				unsigned long size;
> >>  				char *buffer = read_sha1_file(sha1, &type, &size);
> >> -				if (memcmp(buffer, "object ", 7) ||
> >> +				if (!starts_with(buffer, "object ") ||
> >
> > [...]
> >
> >> The original hunks show that the code knows and relies on magic
> >> numbers 7 and 8 very clearly and there are rooms for improvement.
> >
> > Like: what if the file is empty?
> 
> Yes.

I think this one is actually OK. The result of read_sha1_file is NUL-terminated, and we know that starts_with reads byte-by-byte (the prior memcmp is wrong, but only if you care about accessing bytes past the end of the NUL).

That whole piece of code seems silly, though, IMHO. It should be using parse_tag or peel_to_type.

-Peff