From: René Scharfe <l.s.r@web.de>
Date: Sun, 27 Sep 2015 13:24:46 GMT
Subject: Re: [PATCH 60/68] prefer memcpy to strcpy
Message-ID: <5607EE1E.4050304@web.de>
In-Reply-To: <5607EB5D.2050706@web.de>

Am 27.09.2015 um 15:13 schrieb René Scharfe:
> Am 27.09.2015 um 15:06 schrieb Torsten Bögershausen:
>> On 2015-09-27 13.19, René Scharfe wrote:
>>> Am 24.09.2015 um 23:08 schrieb Jeff King:
>>>> When we already know the length of a string (e.g., because
>>>> we just malloc'd to fit it), it's nicer to use memcpy than
>>>> strcpy, as it makes it more obvious that we are not going to
>>>> overflow the buffer (because the size we pass matches the
>>>> size in the allocation).
>>>>
>>>> This also eliminates calls to strcpy, which make auditing
>>>> the code base harder.
>>>>
>>>> Signed-off-by: Jeff King <peff@peff.net>
>>>> ---
>>>>    compat/nedmalloc/nedmalloc.c | 5 +++--
>>>>    fast-import.c                | 5 +++--
>>>>    revision.c                   | 2 +-
>>>>    3 files changed, 7 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/compat/nedmalloc/nedmalloc.c
>>>> b/compat/nedmalloc/nedmalloc.c
>>>> index 609ebba..a0a16eb 100644
>>>> --- a/compat/nedmalloc/nedmalloc.c
>>>> +++ b/compat/nedmalloc/nedmalloc.c
>>>> @@ -957,8 +957,9 @@ char *strdup(const char *s1)
>>>>    {
>>>>        char *s2 = 0;
>>>>        if (s1) {
>>>> -        s2 = malloc(strlen(s1) + 1);
>>>> -        strcpy(s2, s1);
>>>> +        size_t len = strlen(s1) + 1;
>>>> +        s2 = malloc(len);
>>>> +        memcpy(s2, s1, len);
>>>
>>> This leaves the last byte uninitialized; it was set to NUL by
>>> strcpy() before.
>>
>> len is == strlen() +1, which should cover the NUL:
>>
>> 1 byte extra for NUL is allocated,
>> and memcpy will copy NUL from source.
>> (Or do I miss somethong ?)
>
> No, you're right.  Sorry for the noise.
>
> I fully blame this on lack of coffeine because my electric kettle just
> broke. O_o

Thinking a bit more about it (slowly): The choice of the variable name 
might have been a factor as well.  When I see "len" for a string then I 
don't expect it to include the trailing NUL.  "size" would be better 
because I expect it to contain the number of bytes needed to store an 
object.

René