From e2991c80485c646c86f5d80423f9ae983bed120b Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Jun 2015 16:03:09 +0200 Subject: cmd_update_ref(): make logic more straightforward Restructure the code to avoid clearing oldsha1 when oldval is unset. It's value is not needed in that case, so this change makes it more obvious that its initialization is consistent with its later use. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/update-ref.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'builtin/update-ref.c') diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 3d79a46b03..160c7ac1d3 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -408,9 +408,16 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix) die("%s: not a valid SHA1", value); } - hashclr(oldsha1); /* all-zero hash in case oldval is the empty string */ - if (oldval && *oldval && get_sha1(oldval, oldsha1)) - die("%s: not a valid old SHA1", oldval); + if (oldval) { + if (!*oldval) + /* + * The empty string implies that the reference + * must not already exist: + */ + hashclr(oldsha1); + else if (get_sha1(oldval, oldsha1)) + die("%s: not a valid old SHA1", oldval); + } if (no_deref) flags = REF_NODEREF; -- cgit v1.2.3 From 1c03c4d34771db20b78231359caa6fda28e2d9fe Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Jun 2015 16:03:10 +0200 Subject: delete_ref(): use the usual convention for old_sha1 The ref_transaction_update() family of functions use the following convention for their old_sha1 parameters: * old_sha1 == NULL: Don't check the old value at all. * is_null_sha1(old_sha1): Ensure that the reference didn't exist before the transaction. * otherwise: Ensure that the reference had the specified value before the transaction. delete_ref() had a different convention, namely treating is_null_sha1(old_sha1) as "don't care". Change it to adhere to the standard convention to reduce the scope for confusion. Please note that it is now a bug to pass old_sha1=NULL_SHA1 to delete_ref() (because it doesn't make sense to delete a reference that you already know doesn't exist). This is consistent with the behavior of ref_transaction_delete(). Most of the callers of delete_ref() never pass old_sha1=NULL_SHA1 to delete_ref(), and are therefore unaffected by this change. The two exceptions are: * The call in cmd_update_ref(), which passed NULL_SHA1 if the old value passed in on the command line was 0{40} or the empty string. Change that caller to pass NULL in those cases. Arguably, it should be an error to call "update-ref -d" with the old value set to "does not exist", just as it is for the `--stdin` command "delete". But since this usage was accepted until now, continue to accept it. * The call in delete_branches(), which could pass NULL_SHA1 if deleting a broken or symbolic ref. Change it to pass NULL in these cases. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/update-ref.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'builtin/update-ref.c') diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 160c7ac1d3..6763cf1837 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -422,7 +422,13 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix) if (no_deref) flags = REF_NODEREF; if (delete) - return delete_ref(refname, oldval ? oldsha1 : NULL, flags); + /* + * For purposes of backwards compatibility, we treat + * NULL_SHA1 as "don't care" here: + */ + return delete_ref(refname, + (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL, + flags); else return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL, flags, UPDATE_REFS_DIE_ON_ERR); -- cgit v1.2.3