From 581d4e0cdbe9526c122b9d0835f951e478f82448 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Thu, 12 Feb 2015 12:12:12 +0100 Subject: refs: move REF_DELETING to refs.c It is only used internally now. Document it a little bit better, too. Signed-off-by: Michael Haggerty Reviewed-by: Stefan Beller Signed-off-by: Junio C Hamano --- refs.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'refs.c') diff --git a/refs.c b/refs.c index ab2f2a92cd..5e6355c930 100644 --- a/refs.c +++ b/refs.c @@ -34,6 +34,12 @@ static unsigned char refname_disposition[256] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4 }; +/* + * Flag passed to lock_ref_sha1_basic() telling it to tolerate broken + * refs (i.e., because the reference is about to be deleted anyway). + */ +#define REF_DELETING 0x02 + /* * Used as a flag to ref_transaction_delete when a loose ref is being * pruned. -- cgit v1.2.3 From 31e79f0a54e57454a9677eeb8b1108e4f907b8b9 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Thu, 12 Feb 2015 12:12:13 +0100 Subject: refs: remove the gap in the REF_* constant values There is no reason to "reserve" a gap between the public and private flags values. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 5e6355c930..4de13833c8 100644 --- a/refs.c +++ b/refs.c @@ -44,7 +44,8 @@ static unsigned char refname_disposition[256] = { * Used as a flag to ref_transaction_delete when a loose ref is being * pruned. */ -#define REF_ISPRUNING 0x0100 +#define REF_ISPRUNING 0x04 + /* * Try to read one refname component from the front of refname. * Return the length of the component found, or -1 if the component is -- cgit v1.2.3 From fec14ec38ca65b13f9e0fcdb60f27674c6f9af70 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Tue, 17 Feb 2015 18:00:13 +0100 Subject: refs.c: change some "flags" to "unsigned int" Change the following functions' "flags" arguments from "int" to "unsigned int": * ref_transaction_update() * ref_transaction_create() * ref_transaction_delete() * update_ref() * delete_ref() * lock_ref_sha1_basic() Also change the "flags" member in "struct ref_update" to unsigned. Suggested-by: Junio C Hamano Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 4de13833c8..a203e44559 100644 --- a/refs.c +++ b/refs.c @@ -2256,7 +2256,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log) static struct ref_lock *lock_ref_sha1_basic(const char *refname, const unsigned char *old_sha1, const struct string_list *skip, - int flags, int *type_p) + unsigned int flags, int *type_p) { char *ref_file; const char *orig_refname = refname; @@ -2740,14 +2740,14 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err) return 0; } -int delete_ref(const char *refname, const unsigned char *sha1, int delopt) +int delete_ref(const char *refname, const unsigned char *sha1, unsigned int flags) { struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; transaction = ref_transaction_begin(&err); if (!transaction || - ref_transaction_delete(transaction, refname, sha1, delopt, + ref_transaction_delete(transaction, refname, sha1, flags, sha1 && !is_null_sha1(sha1), NULL, &err) || ref_transaction_commit(transaction, &err)) { error("%s", err.buf); @@ -3571,7 +3571,7 @@ int for_each_reflog(each_ref_fn fn, void *cb_data) struct ref_update { unsigned char new_sha1[20]; unsigned char old_sha1[20]; - int flags; /* REF_NODEREF? */ + unsigned int flags; /* REF_NODEREF? */ int have_old; /* 1 if old_sha1 is valid, 0 otherwise */ struct ref_lock *lock; int type; @@ -3644,7 +3644,7 @@ int ref_transaction_update(struct ref_transaction *transaction, const char *refname, const unsigned char *new_sha1, const unsigned char *old_sha1, - int flags, int have_old, const char *msg, + unsigned int flags, int have_old, const char *msg, struct strbuf *err) { struct ref_update *update; @@ -3678,7 +3678,7 @@ int ref_transaction_update(struct ref_transaction *transaction, int ref_transaction_create(struct ref_transaction *transaction, const char *refname, const unsigned char *new_sha1, - int flags, const char *msg, + unsigned int flags, const char *msg, struct strbuf *err) { return ref_transaction_update(transaction, refname, new_sha1, @@ -3688,7 +3688,7 @@ int ref_transaction_create(struct ref_transaction *transaction, int ref_transaction_delete(struct ref_transaction *transaction, const char *refname, const unsigned char *old_sha1, - int flags, int have_old, const char *msg, + unsigned int flags, int have_old, const char *msg, struct strbuf *err) { return ref_transaction_update(transaction, refname, null_sha1, @@ -3697,7 +3697,7 @@ int ref_transaction_delete(struct ref_transaction *transaction, int update_ref(const char *action, const char *refname, const unsigned char *sha1, const unsigned char *oldval, - int flags, enum action_on_err onerr) + unsigned int flags, enum action_on_err onerr) { struct ref_transaction *t; struct strbuf err = STRBUF_INIT; @@ -3781,7 +3781,7 @@ int ref_transaction_commit(struct ref_transaction *transaction, /* Acquire all locks while verifying old values */ for (i = 0; i < n; i++) { struct ref_update *update = updates[i]; - int flags = update->flags; + unsigned int flags = update->flags; if (is_null_sha1(update->new_sha1)) flags |= REF_DELETING; -- cgit v1.2.3 From 8df4e51138781927962438819d79ae3221b527b5 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Tue, 17 Feb 2015 18:00:14 +0100 Subject: struct ref_update: move "have_old" into "flags" Instead of having a separate have_old field, record this boolean value as a bit in the "flags" field. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index a203e44559..e3a2ba8bd7 100644 --- a/refs.c +++ b/refs.c @@ -41,11 +41,17 @@ static unsigned char refname_disposition[256] = { #define REF_DELETING 0x02 /* - * Used as a flag to ref_transaction_delete when a loose ref is being + * Used as a flag in ref_update::flags when a loose ref is being * pruned. */ #define REF_ISPRUNING 0x04 +/* + * Used as a flag in ref_update::flags when old_sha1 should be + * checked. + */ +#define REF_HAVE_OLD 0x08 + /* * Try to read one refname component from the front of refname. * Return the length of the component found, or -1 if the component is @@ -3563,16 +3569,20 @@ int for_each_reflog(each_ref_fn fn, void *cb_data) } /** - * Information needed for a single ref update. Set new_sha1 to the - * new value or to zero to delete the ref. To check the old value - * while locking the ref, set have_old to 1 and set old_sha1 to the - * value or to zero to ensure the ref does not exist before update. + * Information needed for a single ref update. Set new_sha1 to the new + * value or to null_sha1 to delete the ref. To check the old value + * while the ref is locked, set (flags & REF_HAVE_OLD) and set + * old_sha1 to the old value, or to null_sha1 to ensure the ref does + * not exist before update. */ struct ref_update { unsigned char new_sha1[20]; unsigned char old_sha1[20]; - unsigned int flags; /* REF_NODEREF? */ - int have_old; /* 1 if old_sha1 is valid, 0 otherwise */ + /* + * One or more of REF_HAVE_OLD, REF_NODEREF, + * REF_DELETING, and REF_ISPRUNING: + */ + unsigned int flags; struct ref_lock *lock; int type; char *msg; @@ -3666,10 +3676,11 @@ int ref_transaction_update(struct ref_transaction *transaction, update = add_update(transaction, refname); hashcpy(update->new_sha1, new_sha1); - update->flags = flags; - update->have_old = have_old; - if (have_old) + if (have_old) { hashcpy(update->old_sha1, old_sha1); + flags |= REF_HAVE_OLD; + } + update->flags = flags; if (msg) update->msg = xstrdup(msg); return 0; @@ -3785,13 +3796,13 @@ int ref_transaction_commit(struct ref_transaction *transaction, if (is_null_sha1(update->new_sha1)) flags |= REF_DELETING; - update->lock = lock_ref_sha1_basic(update->refname, - (update->have_old ? - update->old_sha1 : - NULL), - NULL, - flags, - &update->type); + update->lock = lock_ref_sha1_basic( + update->refname, + ((update->flags & REF_HAVE_OLD) ? + update->old_sha1 : NULL), + NULL, + flags, + &update->type); if (!update->lock) { ret = (errno == ENOTDIR) ? TRANSACTION_NAME_CONFLICT -- cgit v1.2.3 From 1d147bdff0b8132d3aa53a46df8dbab7b673b796 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Tue, 17 Feb 2015 18:00:15 +0100 Subject: ref_transaction_update(): remove "have_old" parameter Instead, verify the reference's old value if and only if old_sha1 is non-NULL. ref_transaction_delete() will get the same treatment in a moment. Signed-off-by: Michael Haggerty Reviewed-by: Stefan Beller Signed-off-by: Junio C Hamano --- refs.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index e3a2ba8bd7..e88817c623 100644 --- a/refs.c +++ b/refs.c @@ -3654,7 +3654,7 @@ int ref_transaction_update(struct ref_transaction *transaction, const char *refname, const unsigned char *new_sha1, const unsigned char *old_sha1, - unsigned int flags, int have_old, const char *msg, + unsigned int flags, const char *msg, struct strbuf *err) { struct ref_update *update; @@ -3664,9 +3664,6 @@ int ref_transaction_update(struct ref_transaction *transaction, if (transaction->state != REF_TRANSACTION_OPEN) die("BUG: update called for transaction that is not open"); - if (have_old && !old_sha1) - die("BUG: have_old is true but old_sha1 is NULL"); - if (!is_null_sha1(new_sha1) && check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) { strbuf_addf(err, "refusing to update ref with bad name %s", @@ -3676,7 +3673,7 @@ int ref_transaction_update(struct ref_transaction *transaction, update = add_update(transaction, refname); hashcpy(update->new_sha1, new_sha1); - if (have_old) { + if (old_sha1) { hashcpy(update->old_sha1, old_sha1); flags |= REF_HAVE_OLD; } @@ -3693,7 +3690,7 @@ int ref_transaction_create(struct ref_transaction *transaction, struct strbuf *err) { return ref_transaction_update(transaction, refname, new_sha1, - null_sha1, flags, 1, msg, err); + null_sha1, flags, msg, err); } int ref_transaction_delete(struct ref_transaction *transaction, @@ -3702,8 +3699,9 @@ int ref_transaction_delete(struct ref_transaction *transaction, unsigned int flags, int have_old, const char *msg, struct strbuf *err) { - return ref_transaction_update(transaction, refname, null_sha1, - old_sha1, flags, have_old, msg, err); + return ref_transaction_update(transaction, refname, + null_sha1, have_old ? old_sha1 : NULL, + flags, msg, err); } int update_ref(const char *action, const char *refname, @@ -3715,8 +3713,8 @@ int update_ref(const char *action, const char *refname, t = ref_transaction_begin(&err); if (!t || - ref_transaction_update(t, refname, sha1, oldval, flags, - !!oldval, action, &err) || + ref_transaction_update(t, refname, sha1, oldval, + flags, action, &err) || ref_transaction_commit(t, &err)) { const char *str = "update_ref failed for ref '%s': %s"; -- cgit v1.2.3 From fb5a6bb61c215546b94156bbb54d43225424f7d0 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Tue, 17 Feb 2015 18:00:16 +0100 Subject: ref_transaction_delete(): remove "have_old" parameter Instead, verify the reference's old value if and only if old_sha1 is non-NULL. Signed-off-by: Michael Haggerty Reviewed-by: Stefan Beller Signed-off-by: Junio C Hamano --- refs.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index e88817c623..e3c4ab5fd5 100644 --- a/refs.c +++ b/refs.c @@ -2576,7 +2576,7 @@ static void prune_ref(struct ref_to_prune *r) transaction = ref_transaction_begin(&err); if (!transaction || ref_transaction_delete(transaction, r->name, r->sha1, - REF_ISPRUNING, 1, NULL, &err) || + REF_ISPRUNING, NULL, &err) || ref_transaction_commit(transaction, &err)) { ref_transaction_free(transaction); error("%s", err.buf); @@ -2753,8 +2753,9 @@ int delete_ref(const char *refname, const unsigned char *sha1, unsigned int flag transaction = ref_transaction_begin(&err); if (!transaction || - ref_transaction_delete(transaction, refname, sha1, flags, - sha1 && !is_null_sha1(sha1), NULL, &err) || + ref_transaction_delete(transaction, refname, + (sha1 && !is_null_sha1(sha1)) ? sha1 : NULL, + flags, NULL, &err) || ref_transaction_commit(transaction, &err)) { error("%s", err.buf); ref_transaction_free(transaction); @@ -3696,11 +3697,11 @@ int ref_transaction_create(struct ref_transaction *transaction, int ref_transaction_delete(struct ref_transaction *transaction, const char *refname, const unsigned char *old_sha1, - unsigned int flags, int have_old, const char *msg, + unsigned int flags, const char *msg, struct strbuf *err) { return ref_transaction_update(transaction, refname, - null_sha1, have_old ? old_sha1 : NULL, + null_sha1, old_sha1, flags, msg, err); } -- cgit v1.2.3 From f04c5b5522214cd48b39399d1d26b07848fd0e52 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Tue, 17 Feb 2015 18:00:19 +0100 Subject: ref_transaction_create(): check that new_sha1 is valid Creating a reference requires a new_sha1 that is not NULL and not null_sha1. Signed-off-by: Michael Haggerty Reviewed-by: Stefan Beller Signed-off-by: Junio C Hamano --- refs.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'refs.c') diff --git a/refs.c b/refs.c index e3c4ab5fd5..b9cf284f31 100644 --- a/refs.c +++ b/refs.c @@ -3690,6 +3690,8 @@ int ref_transaction_create(struct ref_transaction *transaction, unsigned int flags, const char *msg, struct strbuf *err) { + if (!new_sha1 || is_null_sha1(new_sha1)) + die("BUG: create called without valid new_sha1"); return ref_transaction_update(transaction, refname, new_sha1, null_sha1, flags, msg, err); } -- cgit v1.2.3 From 60294596ba6bf2f63506ffad60b9a94fc04612a1 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Tue, 17 Feb 2015 18:00:20 +0100 Subject: ref_transaction_delete(): check that old_sha1 is not null_sha1 It makes no sense to delete a reference that is already known not to exist. Signed-off-by: Michael Haggerty Reviewed-by: Stefan Beller Signed-off-by: Junio C Hamano --- refs.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'refs.c') diff --git a/refs.c b/refs.c index b9cf284f31..d5bfd11790 100644 --- a/refs.c +++ b/refs.c @@ -3702,6 +3702,8 @@ int ref_transaction_delete(struct ref_transaction *transaction, unsigned int flags, const char *msg, struct strbuf *err) { + if (old_sha1 && is_null_sha1(old_sha1)) + die("BUG: delete called with old_sha1 set to zeros"); return ref_transaction_update(transaction, refname, null_sha1, old_sha1, flags, msg, err); -- cgit v1.2.3 From 16180334015ab44b0310b9d896e554a66c36a1a4 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Tue, 17 Feb 2015 18:00:21 +0100 Subject: ref_transaction_verify(): new function to check a reference's value If NULL is passed to ref_transaction_update()'s new_sha1 parameter, then just verify old_sha1 (under lock) without trying to change the new value of the reference. Use this functionality to add a new function ref_transaction_verify(), which checks the current value of the reference under lock but doesn't change it. Use ref_transaction_verify() in the implementation of "git update-ref --stdin"'s "verify" command to avoid the awkward need to "update" the reference to its existing value. Signed-off-by: Michael Haggerty Reviewed-by: Stefan Beller Signed-off-by: Junio C Hamano --- refs.c | 47 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 8 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index d5bfd11790..1aaba3f7f7 100644 --- a/refs.c +++ b/refs.c @@ -46,11 +46,17 @@ static unsigned char refname_disposition[256] = { */ #define REF_ISPRUNING 0x04 +/* + * Used as a flag in ref_update::flags when the reference should be + * updated to new_sha1. + */ +#define REF_HAVE_NEW 0x08 + /* * Used as a flag in ref_update::flags when old_sha1 should be * checked. */ -#define REF_HAVE_OLD 0x08 +#define REF_HAVE_OLD 0x10 /* * Try to read one refname component from the front of refname. @@ -3577,10 +3583,17 @@ int for_each_reflog(each_ref_fn fn, void *cb_data) * not exist before update. */ struct ref_update { + /* + * If (flags & REF_HAVE_NEW), set the reference to this value: + */ unsigned char new_sha1[20]; + /* + * If (flags & REF_HAVE_OLD), check that the reference + * previously had this value: + */ unsigned char old_sha1[20]; /* - * One or more of REF_HAVE_OLD, REF_NODEREF, + * One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF, * REF_DELETING, and REF_ISPRUNING: */ unsigned int flags; @@ -3665,7 +3678,7 @@ int ref_transaction_update(struct ref_transaction *transaction, if (transaction->state != REF_TRANSACTION_OPEN) die("BUG: update called for transaction that is not open"); - if (!is_null_sha1(new_sha1) && + if (new_sha1 && !is_null_sha1(new_sha1) && check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) { strbuf_addf(err, "refusing to update ref with bad name %s", refname); @@ -3673,7 +3686,10 @@ int ref_transaction_update(struct ref_transaction *transaction, } update = add_update(transaction, refname); - hashcpy(update->new_sha1, new_sha1); + if (new_sha1) { + hashcpy(update->new_sha1, new_sha1); + flags |= REF_HAVE_NEW; + } if (old_sha1) { hashcpy(update->old_sha1, old_sha1); flags |= REF_HAVE_OLD; @@ -3709,6 +3725,19 @@ int ref_transaction_delete(struct ref_transaction *transaction, flags, msg, err); } +int ref_transaction_verify(struct ref_transaction *transaction, + const char *refname, + const unsigned char *old_sha1, + unsigned int flags, + struct strbuf *err) +{ + if (!old_sha1) + die("BUG: verify called with old_sha1 set to NULL"); + return ref_transaction_update(transaction, refname, + NULL, old_sha1, + flags, NULL, err); +} + int update_ref(const char *action, const char *refname, const unsigned char *sha1, const unsigned char *oldval, unsigned int flags, enum action_on_err onerr) @@ -3797,7 +3826,7 @@ int ref_transaction_commit(struct ref_transaction *transaction, struct ref_update *update = updates[i]; unsigned int flags = update->flags; - if (is_null_sha1(update->new_sha1)) + if ((flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1)) flags |= REF_DELETING; update->lock = lock_ref_sha1_basic( update->refname, @@ -3819,8 +3848,9 @@ int ref_transaction_commit(struct ref_transaction *transaction, /* Perform updates first so live commits remain referenced */ for (i = 0; i < n; i++) { struct ref_update *update = updates[i]; + int flags = update->flags; - if (!is_null_sha1(update->new_sha1)) { + if ((flags & REF_HAVE_NEW) && !is_null_sha1(update->new_sha1)) { if (write_ref_sha1(update->lock, update->new_sha1, update->msg)) { update->lock = NULL; /* freed by write_ref_sha1 */ @@ -3836,14 +3866,15 @@ int ref_transaction_commit(struct ref_transaction *transaction, /* Perform deletes now that updates are safely completed */ for (i = 0; i < n; i++) { struct ref_update *update = updates[i]; + int flags = update->flags; - if (update->lock) { + if ((flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1)) { if (delete_ref_loose(update->lock, update->type, err)) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; } - if (!(update->flags & REF_ISPRUNING)) + if (!(flags & REF_ISPRUNING)) string_list_append(&refs_to_delete, update->lock->ref_name); } -- cgit v1.2.3 From 4b7b520b9f761704400a82285d0812fd9e50957f Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Tue, 17 Feb 2015 18:00:22 +0100 Subject: update_ref(): improve documentation Add a docstring for update_ref(), emphasizing its similarity to ref_transaction_update(). Rename its parameters to match those of ref_transaction_update(). Signed-off-by: Michael Haggerty Reviewed-by: Stefan Beller Signed-off-by: Junio C Hamano --- refs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 1aaba3f7f7..8d46b08055 100644 --- a/refs.c +++ b/refs.c @@ -3738,8 +3738,8 @@ int ref_transaction_verify(struct ref_transaction *transaction, flags, NULL, err); } -int update_ref(const char *action, const char *refname, - const unsigned char *sha1, const unsigned char *oldval, +int update_ref(const char *msg, const char *refname, + const unsigned char *new_sha1, const unsigned char *old_sha1, unsigned int flags, enum action_on_err onerr) { struct ref_transaction *t; @@ -3747,8 +3747,8 @@ int update_ref(const char *action, const char *refname, t = ref_transaction_begin(&err); if (!t || - ref_transaction_update(t, refname, sha1, oldval, - flags, action, &err) || + ref_transaction_update(t, refname, new_sha1, old_sha1, + flags, msg, &err) || ref_transaction_commit(t, &err)) { const char *str = "update_ref failed for ref '%s': %s"; -- cgit v1.2.3