Re: [PATCH v7 1/4] sha1_file.c: support reading from a loose object of unknown type
- From
karthik nayak <karthik.188@gmail.com>
- Date
- Apr 5, 2015, 07:52 UTC
- Message-ID
- <5520E9D2.4010603@gmail.com>
- In-Reply-To
- <xmqqiodbdnkn.fsf@gitster.dls.corp.google.com>
On 04/05/2015 01:16 PM, Junio C Hamano wrote:
Show 25 quoted lines
> 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.
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.
Show 13 quoted lines
>
> 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.