Subject: Re: [PATCH v2 04/20] reftable/record: stop using `COPY_ARRAY()`

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 3 Feb 2025 09:40:47 +0100

On Wed, Jan 29, 2025 at 09:46:54AM -0600, Justin Tobler wrote:
> On 25/01/28 09:28AM, Patrick Steinhardt wrote:
> > Drop our use of `COPY_ARRAY()`, replacing it with an open-coded variant
> > thereof. This is done to reduce our dependency on the Git library.
> > 
> > Signed-off-by: Patrick Steinhardt <[email-masked]>
> > ---
> >  reftable/record.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/reftable/record.c b/reftable/record.c
> > index 8919df8a4d..d1664c47ca 100644
> > --- a/reftable/record.c
> > +++ b/reftable/record.c
> > @@ -508,7 +508,8 @@ static int reftable_obj_record_copy_from(void *rec, const void *src_rec,
> >  	if (!obj->offsets)
> >  		return REFTABLE_OUT_OF_MEMORY_ERROR;
> >  	obj->offset_len = src->offset_len;
> > -	COPY_ARRAY(obj->offsets, src->offsets, src->offset_len);
> > +	if (src->offset_len)
> > +		memcpy(obj->offsets, src->offsets, sizeof(*src->offsets) * src->offset_len);
> 
> The `COPY_ARRAY` version uses `st_mutl()` to protect against potential
> overflows of the size parameter. Does this variant need to account for
> such situations as well?

It shouldn't be needed as we already know that the allocation was
successful in the source record, and we're basically just copying things
over. But better be safe than sorry, so I'll add it in.

Patrick