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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 04/05/2015 01:16 PM, Junio C Hamano wrote:
karthik nayak <karthik.188@xxxxxxxxx> 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.


Yes! I do agree with you, If you read my previous reply, I did mention what you said about not having a guarantee that typep and typename are both set and another user might have a bug with this.


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.


Yes! this is a better approach as it will enforce that typep must be set when typename is set.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]