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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> >> --- 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
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]