From: karthik nayak <karthik.188@gmail.com>
Date: Wed, 25 Mar 2015 20:22:31 GMT
Subject: Re: [PATCH v5 1/2] sha1_file.c: support reading from a loose object of unknown type
Message-ID: <55131907.8@gmail.com>
In-Reply-To: <xmqqvbho27ym.fsf@gitster.dls.corp.google.com>



On 03/26/2015 12:57 AM, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
>> +					unsigned long mapsize,
>> +					struct strbuf *header)
>> +{
>> +	unsigned char buffer[32], *cp;
>> +	unsigned long bufsiz = sizeof(buffer);
>> +	int status;
>> +
>> +	status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);
>> +
>> +	if (status) {
>> +		strbuf_add(header, buffer, stream->next_out - buffer);
>> +		return status;
>> +	}
>> +
>> +	do {
>> +		status = git_inflate(stream, 0);
>> +		strbuf_add(header, buffer, stream->next_out - buffer);
>> +		for (cp = buffer; cp < stream->next_out; cp++)
>> +			if (!*cp)
>> +				/* Found the NUL at the end of the header */
>> +				return 0;
>> +		stream->next_out = buffer;
>> +		stream->avail_out = bufsiz;
>> +	} while (status == Z_OK);
>> +	return -1;
>
> A caller of unpack_sha1_header() gets the return from git_inflate();
> I wonder if we should be returning "status" here for consistency,
> perhaps?
>
Although the caller only checks for negative values for failure. For 
consistency it would be better to return status as unpack_sha1_header() 
does.