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

From: karthik nayak <karthik.188@gmail.com>
Date: Mar 7, 2015, 10:04 UTC
Message-ID: <54FACD30.6080005@gmail.com>
In-Reply-To: <xmqq1tl2ylfo.fsf@gitster.dls.corp.google.com>

On 03/07/2015 12:58 AM, Junio C Hamano wrote:

Show 65 quoted lines

> karthik nayak <karthik.188@gmail.com> writes:
>
>>> ... 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?
>
> When I gave you $gmane/264420, I was actually hoping that we do not
> have to have "object-info-literally" helper at all, and instead the
> caller in cat-file that deals with "-t" option can become something
> like this:
> 	
> 	struct object_info oi = { NULL };
> 	struct strbuf typename = STRBUF_INIT;
> 	unsigned flags = LOOKUP_REPLACE_OBJECT;
>
>          if (doing the --literally stuff)
> 		flags |= LOOKUP_LITERALLY;
>
> 	...
>
> 	switch (...) {
> 	case 't':
>          	oi.typename = &typename;
>                  sha1_object_info_extended(sha1, &oi, flags);
> 		if (typename.len) {
>                  	printf("%s\n", typename.buf);
> 			return 0;
> 		}
>                  break;
> 	...
>
> The change illustrated in $gmane/264420 is probably incomplete and
> some calls from the sha1_object_info_extended() after that change
> may still need to be tweaked to pay attention to LOOKUP_LITERALLY
> bit (e.g. parse_sha1_header() may want to learn not to barf when
> seeing an unexpected typename in the header when the caller asks to
> look up "literally").
>

I got confused with $gmane/264420 thanks for clearing that up, also I tried implementing it as follows :

case 't':
	oi.typep = &type;
	oi.typename = &sb;
	sha1_object_info_extended(sha1, &oi, flags);
	if (sb.len) {
		printf("%s\n", sb.buf);
		strbuf_release(&sb);
		return 0;
	} else if (type) {
		printf("%s\n", typename(type));
		return 0;
	}
	break;

This works but I need an else statement to check the type if not getting the type literally, which is because if not called literally the oi.typename is not set, which I will fix now. Also when trying to get the type "literally" it does not call parse_sha1_header() hence we don't need to worry about it handling unexpected typenames.