Re: [PATCH 60/68] prefer memcpy to strcpy
- From
René Scharfe <l.s.r@web.de>
- Date
- Sep 27, 2015, 13:24 UTC
- Message-ID
- <5607EE1E.4050304@web.de>
- In-Reply-To
- <5607EB5D.2050706@web.de>
Am 27.09.2015 um 15:13 schrieb René Scharfe:
Show 47 quoted lines
> 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_oThinking 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é