On Fri, Aug 02, 2024 at 04:24:42PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
> > + } else if (errno == EEXIST) {
> > err = REFTABLE_LOCK_ERROR;
> > - else
> > + goto done;
> > + } else {
> > err = REFTABLE_IO_ERROR;
> > - goto done;
> > + goto done;
> > + }
> > }
> >
> > /*
> > @@ -1270,7 +1308,7 @@ static int stack_compact_range(struct reftable_stack *st,
> > * delete the files after we closed them on Windows, so this needs to
> > * happen first.
> > */
> > - err = reftable_stack_reload_maybe_reuse(st, first < last);
> > + err = reftable_stack_reload_maybe_reuse(st, first_to_replace < last_to_replace);
>
> What is this change about?
>
> No code changed that computes first_to_replace and last_to_replace?
> Perhaps before this step, using first/last and using
> first_to_replace/last_to_replace did not make any difference,
> because we never dealt with a case where we failed to lock any of
> the tables?
>
> I am wondering if this would be helped by a no-op clarification
> before the actual behaviour change, similar to how step 4/8 added
> the nlocks variable.
As we know that we always want to replace the same set of tables, just
with different offsets, it follows that `first - last` is always equal
to `first_to_replace - last_to_replace`.
So this is a no-op change, let me just drop it.
Patrick
Attachment:
signature.asc
Description: PGP signature
