Re: [PATCH 4/7] reftable/stack: stop using `fsync_component()` directly

From: Patrick Steinhardt <ps@pks.im>
Date: Nov 8, 2024, 06:17 UTC
Message-ID: <Zy2s41sv4IlMoISl@pks.im>
In-Reply-To: <6ghxx5fmuzujegducuva77vpybihz6b5cnk75wdgv4pv2knac5@m3trfmg2dk5s>

On Thu, Nov 07, 2024 at 08:09:21PM -0600, Justin Tobler wrote:

Show 26 quoted lines

> On 24/10/23 11:56AM, Patrick Steinhardt wrote:
> [snip]
> > diff --git a/reftable/stack.c b/reftable/stack.c
> > index 9ae716ff375..df4f3237007 100644
> > --- a/reftable/stack.c
> > +++ b/reftable/stack.c
> > @@ -43,17 +42,28 @@ static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st,
> >  	return 0;
> >  }
> >  
> > -static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)
> > +static int stack_fsync(struct reftable_stack *st, int fd)
> >  {
> > -	int *fdp = (int *)arg;
> > -	return write_in_full(*fdp, data, sz);
> > +	if (st->opts.fsync)
> > +		return st->opts.fsync(fd);
> > +	return fsync(fd);
> >  }
> >  
> > -static int reftable_fd_flush(void *arg)
> > +struct fd_writer {
> > +	struct reftable_stack *stack;
> 
> Out of curiousity, from the stack I think we only need the callback in
> the options. Any reason we provide the whole stack here?

I just think that passing around function pointers doesn't make for a good calling convention here, as it hides the fact that it is possible to call this without a callback. But there isn't a reason to pass in the whole stack, it would also be fine to instead pass in e.g. the write options.

I think I'll do that instead.

Patrick