On Thu, 2016-02-18 at 13:07 +0100, Michael Haggerty wrote:
> On 02/18/2016 03:44 AM, David Turner wrote:
> > On Fri, 2016-02-12 at 16:07 +0100, Michael Haggerty wrote:
> > > On 02/05/2016 08:44 PM, David Turner wrote:
> > > > Always handle non-normal (per-worktree or pseudo) refs in the
> > > > files
> > > > backend instead of alternate backends.
> > > >
> > > > Sometimes a ref transaction will update both a per-worktree ref
> > > > and
> > > > a
> > > > normal ref. For instance, an ordinary commit might update
> > > > refs/heads/master and HEAD (or at least HEAD's reflog).
> > > >
> > > > Updates to normal refs continue to go through the chosen
> > > > backend.
> > > >
> > > > Updates to non-normal refs are moved to a separate files
> > > > backend
> > > > transaction.
> > > >
> > > > Signed-off-by: David Turner <dturner@xxxxxxxxxxxxxxxx>
> > > > ---
> > > > refs.c | 81
> > > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > +--
> > > > 1 file changed, 79 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/refs.c b/refs.c
> > > > index 227c018..18ba356 100644
> > > > --- a/refs.c
> > > > +++ b/refs.c
> > > > @@ -9,6 +9,11 @@
> > > > #include "object.h"
> > > > #include "tag.h"
> > > >
> > > > +static const char split_transaction_fail_warning[] = N_(
> > > > + "A ref transaction was split across two refs backends.
> > > > Part of the "
> > > > + "transaction succeeded, but then the update to the per
> > > > -worktree refs "
> > > > + "failed. Your repository may be in an inconsistent
> > > > state.");
> > > > +
> > > > /*
> > > > * We always have a files backend and it is the default.
> > > > */
> > > > @@ -791,6 +796,13 @@ void ref_transaction_free(struct
> > > > ref_transaction *transaction)
> > > > free(transaction);
> > > > }
> > > >
> > > > +static void add_update_obj(struct ref_transaction
> > > > *transaction,
> > > > + struct ref_update *update)
> > > > +{
> > > > + ALLOC_GROW(transaction->updates, transaction->nr + 1,
> > > > transaction->alloc);
> > > > + transaction->updates[transaction->nr++] = update;
> > > > +}
> > > > +
> > > > static struct ref_update *add_update(struct ref_transaction
> > > > *transaction,
> > > > const char *refname)
> > > > {
> > > > @@ -798,8 +810,7 @@ static struct ref_update *add_update(struct
> > > > ref_transaction *transaction,
> > > > struct ref_update *update = xcalloc(1, sizeof(*update)
> > > > +
> > > > len);
> > > >
> > > > memcpy((char *)update->refname, refname, len); /*
> > > > includes
> > > > NUL */
> > > > - ALLOC_GROW(transaction->updates, transaction->nr + 1,
> > > > transaction->alloc);
> > > > - transaction->updates[transaction->nr++] = update;
> > > > + add_update_obj(transaction, update);
> > > > return update;
> > > > }
> > > >
> > > > @@ -1217,11 +1228,38 @@ static int dereference_symrefs(struct
> > > > ref_transaction *transaction,
> > > > return 0;
> > > > }
> > > >
> > > > +/*
> > > > + * Move all non-normal ref updates into a specially-created
> > > > + * files-backend transaction
> > > > + */
> > > > +static int move_abnormal_ref_updates(struct ref_transaction
> > > > *transaction,
> > > > + struct ref_transaction
> > > > *files_transaction,
> > > > + struct strbuf *err)
> > > > +{
> > > > + int i;
> > > > +
> > > > + for (i = 0; i < transaction->nr; i++) {
> > > > + int last;
> > > > + struct ref_update *update = transaction
> > > > ->updates[i];
> > > > +
> > > > + if (ref_type(update->refname) ==
> > > > REF_TYPE_NORMAL)
> > > > + continue;
> > > > +
> > > > + last = --transaction->nr;
> > > > + transaction->updates[i] = transaction
> > > > ->updates[last];
> > > > + add_update_obj(files_transaction, update);
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > >
> > > I think this function is incorrect. The update that was
> > > previously at
> > > transaction->updates[i] never gets checked for abnormality.
> >
> > Yes it does; that's the "update" variable that we just checked.
>
> Sorry, I meant to say "the update that was previously at
> `transaction->updates[transaction->nr - 1]` never gets checked for
> abnormality". Because it gets moved to `transaction->updates[i]`,
> then
> `i` is incremented without checking it.
Oh, right.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html