git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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.

Previous: Junio C HamanoNext: Junio C Hamano
Message 6 of 19 in “cat-file: teach cat-file a '--literally' option”
  1. 0/4 cat-file: teach cat-file a '--literally' optionkarthik nayak, Apr 4, 2015
  2. 1/4 sha1_file.c: support reading from a loose object of unknown typeKarthik Nayak, Apr 4, 2015
  3. Junio C HamanoApr 4, 2015
  4. karthik nayakApr 4, 2015
  5. Junio C HamanoApr 5, 2015
  6. karthik nayakApr 5, 2015
  7. Junio C HamanoApr 5, 2015
  8. karthik nayakApr 7, 2015
  9. 1/4 sha1_file.c: support reading from a loose object of unknown typeKarthik Nayak, Apr 5, 2015
  10. Eric SunshineApr 7, 2015
  11. 2/4 cat-file: teach cat-file a '--literally' optionKarthik Nayak, Apr 4, 2015
  12. Eric SunshineApr 7, 2015
  13. 3/4 cat-file: add documentation for '--literally' option.Karthik Nayak, Apr 4, 2015
  14. Eric SunshineApr 7, 2015
  15. 4/4 t1006: add tests for git cat-file --literallyKarthik Nayak, Apr 4, 2015
  16. Eric SunshineApr 7, 2015
  17. karthik nayakApr 8, 2015
  18. Eric SunshineApr 8, 2015
  19. Karthik NayakApr 9, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.