Re: [PATCH 1/2] sha1_file: Add sha1_object_type_literally and export it.
- From
karthik nayak <karthik.188@gmail.com>
- Date
- Feb 25, 2015, 19:59 UTC
- Message-ID
- <54EE29AD.4060802@gmail.com>
- In-Reply-To
- <1424888526.2968.3.camel@leckie>
On 02/25/2015 11:52 PM, David Turner wrote:
Show 7 quoted lines
> 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?
I got the type from the function definition of map_sha1_file(), which is "void *map_sha1_file(const unsigned char *sha1, unsigned long *size)"
Show 10 quoted lines
>
>> + 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.Do you want me to define it as a preprocessor directive?
Show 10 quoted lines
>
>> + 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?What's possible is when the object type name is greater than 32bytes "hdr" will not be able to hold the whole type, its a tradeoff, I guess I should put a null terminator at the end of "hdr". What do you suggest?
> >