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

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

On 04/05/2015 01:04 AM, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
> > @@ -2586,13 +2649,15 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,
> >               *(oi->disk_sizep) = 0;
> >           if (oi->delta_base_sha1)
> >               hashclr(oi->delta_base_sha1);
> > +        if (oi->typename)
> > +            strbuf_addstr(oi->typename, typename(co->type));
> >           oi->whence = OI_CACHED;
> >           return 0;
> >       }
>
> Just before the pre-context of this hunk, there is this bit:
>
>     if (oi->typep)
>         *(oi->typep) = co->type;
>
> which tells me that the callers of this function is allowed to pass
> a NULL in oi->typep when they are not interested in the type of the
> object.
>
> >       if (!find_pack_entry(real, &e)) {
> >           /* Most likely it's a loose object. */
> > -        if (!sha1_loose_object_info(real, oi)) {
> > +        if (!sha1_loose_object_info(real, oi, flags)) {
> >               oi->whence = OI_LOOSE;
> >               return 0;
> >           }
> > @@ -2616,6 +2681,8 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,
> >           oi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||
> >                        rtype == OBJ_OFS_DELTA);
> >       }
> > +    if (oi->typename)
> > +        strbuf_addstr(oi->typename, typename(*oi->typep));
>
> So, it makes me wonder what guarantee we have that this does not
> dereference a NULL here.
>
As per my code, oi->typename is only pointing to something when oi->typep
is ( As oi->typename is currently only used in cat-file.c).
But what you're saying also is true, there is no other guarantee, as a user may
set oi->typename to point to a struct strbuf and leave out oi->typep.

  if (oi->typename && oi->typep)
          strbuf_addstr(oi->typename, typename(*oi->typep));

This should suffice. Do you want me to re-roll this?