Re: [PATCH v8 1/4] sha1_file.c: support reading from a loose object of unknown type

[karthik nayak <karthik.188@gmail.com>]

On 04/18/2015 05:01 AM, Eric Sunshine wrote:
> On Wed, Apr 15, 2015 at 12:59 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> Update sha1_loose_object_info() to optionally allow it to read
>> from a loose object file of unknown/bogus type; as the function
>> usually returns the type of the object it read in the form of enum
>> for known types, add an optional "typename" field to receive the
>> name of the type in textual form and a flag to indicate the reading
>> of a loose object file of unknown/bogus type.
>> [...]
>> ---
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 980ce6b..267399d 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -2522,13 +2575,15 @@ struct packed_git *find_sha1_pack(const unsigned char *sha1,
>>   }
>>
>>   static int sha1_loose_object_info(const unsigned char *sha1,
>> -                                 struct object_info *oi)
>> +                                 struct object_info *oi,
>> +                                 int flags)
>>   {
>> -       int status;
>> -       unsigned long mapsize, size;
>> +       int status = 0;
>> +       unsigned long mapsize;
>>          void *map;
>>          git_zstream stream;
>>          char hdr[32];
>> +       struct strbuf hdrbuf = STRBUF_INIT;
>>
>>          if (oi->delta_base_sha1)
>>                  hashclr(oi->delta_base_sha1);
>> @@ -2555,17 +2610,26 @@ static int sha1_loose_object_info(const unsigned char *sha1,
>>                  return -1;
>>          if (oi->disk_sizep)
>>                  *oi->disk_sizep = mapsize;
>> -       if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
>> -               status = error("unable to unpack %s header",
>> -                              sha1_to_hex(sha1));
>> -       else if ((status = parse_sha1_header(hdr, &size)) < 0)
>> -               status = error("unable to parse %s header", sha1_to_hex(sha1));
>> -       else if (oi->sizep)
>> -               *oi->sizep = size;
>> +       if ((flags & LOOKUP_LITERALLY)) {
>> +               if (unpack_sha1_header_to_strbuf(&stream, map, mapsize, &hdrbuf) < 0)
>> +                       status = error("unable to unpack %s header with --literally",
>> +                                      sha1_to_hex(sha1));
>> +               else if ((status = parse_sha1_header_extended(hdrbuf.buf, oi, flags)) < 0)
>> +                       status = error("unable to parse %s header with --literally",
>> +                                      sha1_to_hex(sha1));
>> +       } else {
>> +               if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
>> +                       status = error("unable to unpack %s header",
>> +                                      sha1_to_hex(sha1));
>> +               else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)
>> +                       status = error("unable to parse %s header", sha1_to_hex(sha1));
>> +       }
>>          git_inflate_end(&stream);
>>          munmap(map, mapsize);
>> -       if (oi->typep)
>> +       if (status && oi->typep)
>>                  *oi->typep = status;
>> +       if (hdrbuf.buf)
>> +               strbuf_release(&hdrbuf);
>
> Why is strbuf_release() protected by a conditional rather than being
> called unconditionally? Am I missing something obvious?
No, you're right.
>
>>          return 0;
>>   }
>>