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