Daniel Barkalow <barkalow@xxxxxxxxxxxx> writes:
> It might be wise to make "default" the return NULL case, and list the two
> okay cases explicitly, so it doesn't need to be changed if
> check_ref_format() someday gets additional "okay for some purposes"
> values.
Sounds sensible.
> Aside from that, it looks good, except that builtin-send-pack.c and
> fast-import.c should probably use the symbolic constants, too. (All other
> callers only check whether the value is true or not).
Also sensible.
-- >8 --
Update callers of check_ref_format()
This updates send-pack and fast-import to use symbolic constants
for checking the return values from check_ref_format(), and also
futureproof the logic in lock_any_ref_for_update() to explicitly
name the case that is usually considered an error but is Ok for
this particular use.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
builtin-send-pack.c | 10 ++++++----
fast-import.c | 5 +++--
refs.c | 6 +++---
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 25ae1fe..8afb1d0 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -541,10 +541,12 @@ static void verify_remote_names(int nr_heads, const char **heads)
remote = remote ? (remote + 1) : heads[i];
switch (check_ref_format(remote)) {
case 0: /* ok */
- case -2: /* ok but a single level -- that is fine for
- * a match pattern.
- */
- case -3: /* ok but ends with a pattern-match character */
+ case CHECK_REF_FORMAT_ONELEVEL:
+ /* ok but a single level -- that is fine for
+ * a match pattern.
+ */
+ case CHECK_REF_FORMAT_WILDCARD:
+ /* ok but ends with a pattern-match character */
continue;
}
die("remote part of refspec is not a valid name in %s",
diff --git a/fast-import.c b/fast-import.c
index 4646c05..74597c9 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -642,8 +642,9 @@ static struct branch *new_branch(const char *name)
if (b)
die("Invalid attempt to create duplicate branch: %s", name);
switch (check_ref_format(name)) {
- case 0: break; /* its valid */
- case -2: break; /* valid, but too few '/', allow anyway */
+ case 0: break; /* its valid */
+ case CHECK_REF_FORMAT_ONELEVEL:
+ break; /* valid, but too few '/', allow anyway */
default:
die("Branch name doesn't conform to GIT standards: %s", name);
}
diff --git a/refs.c b/refs.c
index 7484a46..58f6d17 100644
--- a/refs.c
+++ b/refs.c
@@ -822,10 +822,10 @@ 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)
{
switch (check_ref_format(ref)) {
- case CHECK_REF_FORMAT_ERROR:
- case CHECK_REF_FORMAT_WILDCARD:
- return NULL;
default:
+ return NULL;
+ case 0:
+ case CHECK_REF_FORMAT_ONELEVEL:
return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
}
}
-
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
