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

[Junio C Hamano <gitster@pobox.com>]

karthik nayak <karthik.188@gmail.com> writes:

>> 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?

I'd rather avoid the thinking along the lines of "at this moment,
there happens to be only one caller that asks for typename and the
caller also sets typep, so we will be safe as long as we make sure
the caller passed typep before giving him typename back".

Somebody else may write new code that wants to learn the typename,
forgets to set typep, calls into this codepath, and ends up
scratching his head wondering why the typename string is returned to
him.  Surely the code may not crash at the new code you wrote, but
you are not helping him.

If it semantically does not make sense to ask for the typename
without asking for the type code, then we can and should make that
as a new calling convention _all_ callers must follow.

In other words, I think it is better to have

	if (oi->typename && !oi->typep)
		die("BUG");

somewhere near the beginning of the callchain that takes oi; that
will ensure all callers understand the rule.