On Tuesday, January 26, 2010 at 12:47:02 (-0800) Junio C Hamano writes:
>Bill Lear <rael@xxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html