Am 22.12.21 um 23:50 schrieb Junio C Hamano:
> "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> From: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
>>
>> Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
>> ---
>> reftable/record.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/reftable/record.c b/reftable/record.c
>> index fbaa1fbef56..423e687b220 100644
>> --- a/reftable/record.c
>> +++ b/reftable/record.c
>> @@ -126,7 +126,8 @@ static int encode_string(char *str, struct string_view s)
>> string_view_consume(&s, n);
>> if (s.len < l)
>> return -1;
>> - memcpy(s.buf, str, l);
>> + if (l)
>> + memcpy(s.buf, str, l);
>> string_view_consume(&s, l);
>>
>> return start.len - s.len;
>> @@ -153,7 +154,9 @@ int reftable_encode_key(int *restart, struct string_view dest,
>>
>> if (dest.len < suffix_len)
>> return -1;
>> - memcpy(dest.buf, key.buf + prefix_len, suffix_len);
>> +
>> + if (suffix_len)
>> + memcpy(dest.buf, key.buf + prefix_len, suffix_len);
>> string_view_consume(&dest, suffix_len);
>>
>> return start.len - dest.len;
>> @@ -569,7 +572,8 @@ static int reftable_obj_record_decode(void *rec, struct strbuf key,
>> uint64_t last;
>> int j;
>> r->hash_prefix = reftable_malloc(key.len);
>> - memcpy(r->hash_prefix, key.buf, key.len);
>> + if (key.len)
>> + memcpy(r->hash_prefix, key.buf, key.len);
>> r->hash_prefix_len = key.len;
>>
>> if (val_type == 0) {
>
> I am not sure why any of these are needed.
>
> For a code path that involves a <ptr, len> pair, where ptr is lazily
> allocated only when we have contents to store, i.e. ptr can remain
> NULL until len becomes non-zero, memcpy(dst, ptr, len) can become a
> problem as the standard requires ptr to be a valid even when len is
> 0 (ISO/IEC 9899:1999, 7.21.1 String function conventions, paragraph
> 2), but none of these three calls to memcpy() do any such thing.
>
> Puzzled.
I don't know about the first two, but in the third case dst (i.e.
r->hash_prefix) might be NULL if key.len == 0, reftable_malloc is malloc
(which it is, because reftable_set_alloc is never called) and malloc(0)
returns NULL (which it might do according to
https://www.man7.org/linux/man-pages/man3/malloc.3.html).
malloc can return NULL on failure, too, of course, and none of the
reftable_malloc callers check for that. That seems a bit too
optimistic.
René
