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

[Junio C Hamano <gitster@pobox.com>]

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?