lock_any_ref_for_update(): reject wildcard return from check_ref_format

Recent check_ref_format() returns -3 as well as -1 (general
error) and -2 (less than two levels).  The caller was explicitly
checking for -1, to allow "HEAD" but still needed to disallow
bogus refs.

This introduces symbolic constants for the return values from
check_ref_format() to make them read better and more
meaningful.  Normal ref creation codepath can still treat
non-zero return values as errors.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs.c b/refs.c
index 759924d0c93621d72918f29145ae0377f34e8e0c..7484a46d6834b4c85dd81f1f38596bc3be087426 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -613,32 +613,37 @@ int check_ref_format(const char *ref)
                while ((ch = *cp++) == '/')
                        ; /* tolerate duplicated slashes */
                if (!ch)
-                       return -1; /* should not end with slashes */
+                       /* should not end with slashes */
+                       return CHECK_REF_FORMAT_ERROR;
 
                /* we are at the beginning of the path component */
                if (ch == '.')
-                       return -1;
+                       return CHECK_REF_FORMAT_ERROR;
                bad_type = bad_ref_char(ch);
                if (bad_type) {
-                       return (bad_type == 2 && !*cp) ? -3 : -1;
+                       return (bad_type == 2 && !*cp)
+                               ? CHECK_REF_FORMAT_WILDCARD
+                               : CHECK_REF_FORMAT_ERROR;
                }
 
                /* scan the rest of the path component */
                while ((ch = *cp++) != 0) {
                        bad_type = bad_ref_char(ch);
                        if (bad_type) {
-                               return (bad_type == 2 && !*cp) ? -3 : -1;
+                               return (bad_type == 2 && !*cp)
+                                       ? CHECK_REF_FORMAT_WILDCARD
+                                       : CHECK_REF_FORMAT_ERROR;
                        }
                        if (ch == '/')
                                break;
                        if (ch == '.' && *cp == '.')
-                               return -1;
+                               return CHECK_REF_FORMAT_ERROR;
                }
                level++;
                if (!ch) {
                        if (level < 2)
-                               return -2; /* at least of form "heads/blah" */
-                       return 0;
+                               return CHECK_REF_FORMAT_ONELEVEL;
+                       return CHECK_REF_FORMAT_OK;
                }
        }
 }
@@ -816,9 +821,13 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
 
 struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)
 {
-       if (check_ref_format(ref) == -1)
+       switch (check_ref_format(ref)) {
+       case CHECK_REF_FORMAT_ERROR:
+       case CHECK_REF_FORMAT_WILDCARD:
                return NULL;
-       return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
+       default:
+               return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
+       }
 }
 
 static struct lock_file packlock;

diff --git a/refs.h b/refs.h
index 9dc8aa01d181dbdbf1c8f643a1bd7de1d311ffa3..9cd16f82956d89f2800827046151abd7866fb9da 100644 (file)
--- a/refs.h
+++ b/refs.h
@@ -52,7 +52,10 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data);
  */
 extern int for_each_reflog(each_ref_fn, void *);
 
-/** Returns 0 if target has the right format for a ref. **/
+#define CHECK_REF_FORMAT_OK 0
+#define CHECK_REF_FORMAT_ERROR (-1)
+#define CHECK_REF_FORMAT_ONELEVEL (-2)
+#define CHECK_REF_FORMAT_WILDCARD (-3)
 extern int check_ref_format(const char *target);
 
 /** rename ref, return 0 on success **/