Re: [PATCH v2 02/11] reftable: consistently pass write opts as value

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

On Fri, May 10, 2024 at 02:11:55PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > We sometimes pass the refatble write options as value and sometimes as a
> > pointer. This is quite confusing and makes the reader wonder whether the
> > options get modified sometimes.
> >
> > In fact, `reftable_new_writer()` does cause the caller-provided options
> > to get updated when some values aren't set up. This is quite unexpected,
> > but didn't cause any harm until now.
> >
> > Refactor the code to consistently pass the options as a value so that
> > they are local to the subsystem they are being passed into so that we
> > can avoid weirdness like this.
> 
> Turning pass-by-reference to pass-by-value of a large structure is a
> rather huge hammer to ensure that the structure is not modified
> (qualifying the pointer with "const" is).  Consistency is good, but
> I am not sure offhand if this is making things consistent in a good
> way.

Yeah, I've also been a bit "meh" about this. I'm happy to convert this
to all be const pointers instead.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]