From: David Turner <dturner@twopensource.com>
Date: Wed, 25 Feb 2015 18:22:06 GMT
Subject: Re: [PATCH 1/2] sha1_file: Add sha1_object_type_literally and export it.
Message-ID: <1424888526.2968.3.camel@leckie>
In-Reply-To: <1424862460-13514-1-git-send-email-karthik.188@gmail.com>

On Wed, 2015-02-25 at 16:37 +0530, Karthik Nayak wrote:
> +	unsigned long mapsize;
> ...
> +	map = map_sha1_file(sha1, &mapsize);

I know this is a pre-existing issue, but I'm not sure "unsigned long" is
the right type here.  Shouldn't it be a size_t?  

> +	if (!map)
> +		return -1;
> +	if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
> +		status = error("unable to unpack %s header",
> +			       sha1_to_hex(sha1));
> +
> +	for (i = 0; i < 32; i++) {

This number should probably be a constant.

> +		if (hdr[i] == ' ') {
> +			type[i] = '\0';
> +			break;
> +		}
> +		type[i] = hdr[i];
> +	}

type might end up without a trailing \0 here in the case where hdr has
no space in it.  Is this possible?