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

Re: [PATCH 1/4] Add xmallocz()

From
Bill Lear <rael@zopyra.com>
Date
Jan 26, 2010, 20:56 UTC
Message-ID
<19295.22246.788034.159299@blake.zopyra.com>
In-Reply-To
<7v1vhczj95.fsf@alter.siamese.dyndns.org>
On Tuesday, January 26, 2010 at 12:47:02 (-0800) Junio C Hamano writes:
Show 36 quoted lines
>Bill Lear <rael@zopyra.com> writes:
>
>> On Tuesday, January 26, 2010 at 20:24:12 (+0200) Ilari Liusvaara writes:
>>>Add routine for allocating NUL-terminated memory block without risking
>>>integer overflow in addition of +1 for NUL byte.
>>>...
>>> void *xmemdupz(const void *data, size_t len)
>>> {
>>>-	char *p = xmalloc(len + 1);
>>>+	char *p = xmallocz(len);
>>> 	memcpy(p, data, len);
>>> 	p[len] = '\0';
>>> 	return p;
>>
>> Do you need the statement
>>
>>  	p[len] = '\0';
>>
>> any longer in the above?  If not, could you just do this:
>>
>> void *xmemdupz(const void *data, size_t len)
>> {
>> 	return memcpy(xmallocz(len), data, len);
>> }
>
>I think the intention to name it xmallocz() was "This is to allocate
>buffer to hold 'len' bytes worth of stuff, and between the caller and this
>function the buffer is arranged to be NUL terminated".  Even though none
>of the existing callers of xmalloc() expected the function to do the NUL
>termination (hence they do NUL termination themselves), I _think_ Ilari
>made the function to do this because its name now ends with "z" that hints
>the callers such a NUL-termination might happen inside the function.
>
>I'd rather see the function lose the NUL termination; if that makes the
>behaviour inconsistent with its name, perhaps it is better to rename the
>function; perhaps xmalloc1() to denote that it overallocates by one?

Why have xmallocz/xmalloc1 lose the NUL termination? Is it because some call sites don't need the NUL termination? [I spotted one, I think, in the patch series...].

Bill
Previous: Ilari LiusvaaraNext: Ilari Liusvaara
Message 7 of 13 in “Fix various integer overflows”
  1. 0/4 Fix various integer overflowsIlari Liusvaara, Jan 26, 2010
  2. 1/4 Add xmallocz()Ilari Liusvaara, Jan 26, 2010
  3. Bill LearJan 26, 2010
  4. Junio C HamanoJan 26, 2010
  5. Junio C HamanoJan 26, 2010
  6. Ilari LiusvaaraJan 26, 2010
  7. Bill LearJan 26, 2010
  8. 2/4 Fix integer overflow in patch_delta()Ilari Liusvaara, Jan 26, 2010
  9. 3/4 Fix integer overflow in unpack_sha1_rest()Ilari Liusvaara, Jan 26, 2010
  10. 4/4 Fix integer overflow in unpack_compressed_entry()Ilari Liusvaara, Jan 26, 2010
  11. Junio C HamanoJan 26, 2010
  12. Stephen R. van den BergJan 27, 2010
  13. Ilari LiusvaaraJan 27, 2010

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.