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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 26, 2010, 20:47 UTC
Message-ID: <7v1vhczj95.fsf@alter.siamese.dyndns.org>
In-Reply-To: <19295.21148.182245.516321@blake.zopyra.com>

Bill Lear <rael@zopyra.com> writes:

Show 22 quoted lines

> 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?