Re: [PATCH 10/11] ref_transaction_verify(): new function to check a reference's value
On 02/09/2015 07:50 PM, Stefan Beller wrote: On Sun, Feb 8, 2015 at 8:14 AM, Michael Haggerty mhag...@alum.mit.edu wrote: /* - * Add a reference update to transaction. new_sha1 is the value that - * the reference should have after the update, or null_sha1 if it should - * be deleted. If old_sha1 is non-NULL, then it the value - * that the reference should have had before the update, or null_sha1 if - * it must not have existed beforehand. - * Function returns 0 on success and non-zero on failure. A failure to update - * means that the transaction as a whole has failed and will need to be - * rolled back. + * Add a reference update to transaction. new_sha1 is the value that + * the reference should have after the update, or null_sha1 if it + * should be deleted. If new_sha1 is NULL, then the reference is not + * changed at all. old_sha1 is the value that the reference must have + * before the update, or null_sha1 if it must not have existed + * beforehand. The old value is checked after the lock is taken to + * prevent races. If the old value doesn't agree with old_sha1, the + * whole transaction fails. If old_sha1 is NULL, then the previous + * value is not checked. + * + * Return 0 on success and non-zero on failure. Any failure in the + * transaction means that the transaction as a whole has failed and + * will need to be rolled back. Do we need to be explicit about having to rollback everything in each ref_transaction_* function documentation? Thanks for the suggestion. I just added a new commit that moves this (and more) information to the introductory comment above these four functions' declarations, so that it doesn't have to be repeated for each function. It will be in the re-roll. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/11] ref_transaction_verify(): new function to check a reference's value
On Sun, Feb 8, 2015 at 8:14 AM, Michael Haggerty mhag...@alum.mit.edu wrote: /* - * Add a reference update to transaction. new_sha1 is the value that - * the reference should have after the update, or null_sha1 if it should - * be deleted. If old_sha1 is non-NULL, then it the value - * that the reference should have had before the update, or null_sha1 if - * it must not have existed beforehand. - * Function returns 0 on success and non-zero on failure. A failure to update - * means that the transaction as a whole has failed and will need to be - * rolled back. + * Add a reference update to transaction. new_sha1 is the value that + * the reference should have after the update, or null_sha1 if it + * should be deleted. If new_sha1 is NULL, then the reference is not + * changed at all. old_sha1 is the value that the reference must have + * before the update, or null_sha1 if it must not have existed + * beforehand. The old value is checked after the lock is taken to + * prevent races. If the old value doesn't agree with old_sha1, the + * whole transaction fails. If old_sha1 is NULL, then the previous + * value is not checked. + * + * Return 0 on success and non-zero on failure. Any failure in the + * transaction means that the transaction as a whole has failed and + * will need to be rolled back. Do we need to be explicit about having to rollback everything in each ref_transaction_* function documentation? I like the new wording (first paragraph) of this function documentation. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/11] 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 mhag...@alum.mit.edu
---
builtin/update-ref.c | 7 ++-
refs.c | 47 +++
refs.h | 34 ++
3 files changed, 67 insertions(+), 21 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 76b8aef..a2eedba 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -282,7 +282,6 @@ static const char *parse_cmd_verify(struct ref_transaction
*transaction,
{
struct strbuf err = STRBUF_INIT;
char *refname;
- unsigned char new_sha1[20];
unsigned char old_sha1[20];
refname = parse_refname(input, next);
@@ -293,13 +292,11 @@ static const char *parse_cmd_verify(struct
ref_transaction *transaction,
PARSE_SHA1_OLD))
hashclr(old_sha1);
- hashcpy(new_sha1, old_sha1);
-
if (*next != line_termination)
die(verify %s: extra input: %s, refname, next);
- if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
- update_flags, msg, err))
+ if (ref_transaction_verify(transaction, refname, old_sha1,
+ update_flags, err))
die(%s, err.buf);
update_flags = 0;
diff --git a/refs.c b/refs.c
index 85815d7..0da680e 100644
--- a/refs.c
+++ b/refs.c
@@ -47,10 +47,16 @@ 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.
@@ -3578,10 +3584,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_IS_PRUNING:
*/
int flags;
@@ -3666,7 +3679,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);
@@ -3674,7 +3687,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;
@@ -3710,6 +3726,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,
+ 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,
int flags, enum action_on_err onerr)
@@ -3798,7 +3827,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
int flags = update-flags;
- if
