On 03/06/2015 05:15 AM, Junio C Hamano wrote:
Karthik Nayak <karthik....@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);
I thought I could get the calling function "cat_one_file()" to send
the address to a struct strbuf. Like this ..

struct strbuf sb = STRBUF_INIT;
length = sha1_object_info_literally(sha1, &sb);
if (length < 0)
die("git cat-file --literally -t %s: failed",
            obj_name);
printf("%s\n", sb.buf);
strbuf_release(&sb);
return 0;

What do you think? Is this ok?

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.



>
I also missed a part where the object given was a packed object.
eg : git cat-file -t --literally HEAD~2
And since I missed that out, it wasnt copying the type to oi.typename,
and oi.typename would end up being empty, I found this while i was
using gdb.

Didn't CC the mailing list the first time, sorry.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to