Re: [PATCH v3 2/3] sha1_file: implement changes for "cat-file --literally -t"

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +const char *sha1_object_info_literally(const unsigned char *sha1)
>> +{
>> +	enum object_type type;
>> +	struct strbuf sb = STRBUF_INIT;
>> +	struct object_info oi = {NULL};
>> +
>> +	oi.typename = &sb;
>> +	oi.typep = &type;
>> +	if (sha1_object_info_extended(sha1, &oi, LOOKUP_LITERALLY) < 0)
>> +		return NULL;
>> +	if (*oi.typep > 0) {
>> +		strbuf_release(oi.typename);
>> +		return typename(*oi.typep);
>> +	}
>> +	return oi.typename->buf;
>> +}
>
> After calling this function to ask the textual type of an object,
> should the caller free the result it obtains from this function?
>
> oi.typename points at the strbuf on stack and its buf member points
> at an allocated piece of memory.  That must be freed.
>
> On the other hand, typename(*oi.typep) is a pointer into static
> piece of memory, which must never be freed.
>
> This patch introduces this function without introducing any caller,
> which makes it unnecessarily harder to judge if this problem is
> caused by choosing a wrong calling convention, and/or if so what
> better calling convention can be used to correct the problem, but
> without looking at the caller that (presumably) will be introduced
> in a later patch, I suspect that the caller should supply a pointer
> to struct object_info, i.e. something along these lines:
>
>      struct object_info oi = { NULL };
>      struct strbuf sb = STRBUF_INIT;
>      enum object_type type;
>
>      ...
>
>      oi.typename = &sb;
>      sha1_object_info_literally(sha1, &oi);
>      if (!sb.len)
>          that is an error;
>      else
>          use sb.buf as the name;
>
>      strbuf_release(&sb);

>
> As sha1_object_info_extended() takes oi and fills oi.typename when
> it is supplied for _all_ types, not just the bogus ones, a caller of
> that function, including sha1_object_info_literally() and its
> caller, shouldn't have to worry about "is that a known one?  then
> use typename() to convert the enum type to a string.  Otherwise use
> the oi.typename->buf" at all, I would think.
>
>
>