From: Bill Lear <rael@zopyra.com>
Date: Tue, 26 Jan 2010 20:56:06 GMT
Subject: Re: [PATCH 1/4] Add xmallocz()
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:
>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