Re: [PATCH v5 16/16] reftable: be more paranoid about 0-length memcpy calls
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 22, 2021, 22:50 UTC
- Message-ID
- <xmqq1r24gsph.fsf@gitster.g>
- In-Reply-To
- <e16bf0c5212ae85daa0d6aa2c78d551824b542bd.1640199396.git.gitgitgadget@gmail.com>
"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 42 quoted lines
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> 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.