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

Re: [PATCH 1/6] path.c: allocate enough memory for string

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 30, 2016, 17:16 UTC
Message-ID
<xmqqr3erubs2.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<CAGZ79kb6dsaSxQ+HaYODBurnFz3MbsisSNkdrckFZLvSCvS-JQ@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
Show 23 quoted lines
> On Tue, Mar 29, 2016 at 5:57 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Tue, Mar 29, 2016 at 8:38 PM, Stefan Beller <sbeller@google.com> wrote:
>>> `strlen` returns the length of a string without the terminating null byte.
>>> To make sure enough memory is allocated we need to pass `strlen(..) + 1`
>>> to the allocation function.
>>>
>>> Signed-off-by: Stefan Beller <sbeller@google.com>
>>> ---
>>> diff --git a/path.c b/path.c
>>> @@ -155,7 +155,7 @@ static struct trie *make_trie_node(const char *key, void *value)
>>>         struct trie *new_node = xcalloc(1, sizeof(*new_node));
>>>         new_node->len = strlen(key);
>>>         if (new_node->len) {
>>> -               new_node->contents = xmalloc(new_node->len);
>>> +               new_node->contents = xmalloc(new_node->len + 1);
>>>                 memcpy(new_node->contents, key, new_node->len);
>>
>> Huh? This is a trie. It never accesses 'contents' as a NUL-terminated
>> string. Plus, no NUL is ever even copied, thus this is just
>> overallocating. How is this an improvement?
>
> By using strlen, I assumed it was a standard C string.
> I missed that, though.

You took hint from a wrong place. You are auditing the destination buffer, so the correct place to take hint from is the memcpy() that touches the destination.

Previous: Stefan BellerNext: Stefan Beller
Message 6 of 24 in “Some cleanups”
  1. 0/6 Some cleanupsStefan Beller, Mar 30, 2016
  2. 1/6 path.c: allocate enough memory for stringStefan Beller, Mar 30, 2016
  3. Junio C HamanoMar 30, 2016
  4. Eric SunshineMar 30, 2016
  5. Stefan BellerMar 30, 2016
  6. Junio C HamanoMar 30, 2016
  7. 2/6 imap-send.c, cram: allocate enough memory for null terminated stringStefan Beller, Mar 30, 2016
  8. Eric SunshineMar 30, 2016
  9. Jeff KingMar 30, 2016
  10. 3/6 notes: don't leak memory in git_config_get_notes_strategyStefan Beller, Mar 30, 2016
  11. Eric SunshineMar 30, 2016
  12. Jeff KingMar 30, 2016
  13. Junio C HamanoMar 30, 2016
  14. 4/6 abbrev_sha1_in_line: don't leak memoryStefan Beller, Mar 30, 2016
  15. Jeff KingMar 30, 2016
  16. Eric SunshineMar 30, 2016
  17. Jeff KingMar 30, 2016
  18. Junio C HamanoMar 30, 2016
  19. Jeff KingMar 30, 2016
  20. 5/6 bundle: don't leak an fd in case of early returnStefan Beller, Mar 30, 2016
  21. Jeff KingMar 30, 2016
  22. Junio C HamanoMar 30, 2016
  23. 6/6 credential-cache, send_request: close fd when doneStefan Beller, Mar 30, 2016
  24. Jeff KingMar 30, 2016

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.