From 3809633d0adb77b02ba8cfe87578134e6a30f54d Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Tue, 18 Mar 2025 18:50:18 -0400 Subject: refspec: treat 'fetch' as a Boolean value Since 6d4c057859 (refspec: introduce struct refspec, 2018-05-16), we have macros called REFSPEC_FETCH and REFSPEC_PUSH. This confusingly suggests that we might introduce other modes in the future, which, while possible, is highly unlikely. But these values are treated as a Boolean, and stored in a struct field called 'fetch'. So the following: if (refspec->fetch == REFSPEC_FETCH) { ... } , and if (refspec->fetch) { ... } are equivalent. Let's avoid renaming the Boolean values "true" and "false" here and remove the two REFSPEC_ macros mentioned above. Since this value is truly a Boolean and will only ever take on a value of 0 or 1, we can declare it as a single bit unsigned field. In practice this won't shrink the size of 'struct refspec', but it more clearly indicates the intent. Note that this introduces some awkwardness like: refspec_item_init_or_die(&spec, refspec, 1); , where it's unclear what the final "1" does. This will be addressed in the following commits. Signed-off-by: Taylor Blau Acked-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/pull.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin/pull.c') diff --git a/builtin/pull.c b/builtin/pull.c index 9c4a00620a..8bbfcce729 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -738,7 +738,7 @@ static const char *get_tracking_branch(const char *remote, const char *refspec) const char *spec_src; const char *merge_branch; - refspec_item_init_or_die(&spec, refspec, REFSPEC_FETCH); + refspec_item_init_or_die(&spec, refspec, 1); spec_src = spec.src; if (!*spec_src || !strcmp(spec_src, "HEAD")) spec_src = "HEAD"; -- cgit v1.3 From ec6829e4849feb7b0343940e00896055027b06eb Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Tue, 18 Mar 2025 18:50:24 -0400 Subject: refspec: remove refspec_item_init_or_die() There are two callers of this function, which ensures that a dispatched call to refspec_item_init() does not fail. In the following commit, we're going to add fetch/push-specific variants of refspec_item_init(), which will turn one function into two. To avoid introducing yet another pair of new functions (such as refspec_item_init_push_or_die() and refspec_item_init_fetch_or_die()), let's remove the thin wrapper entirely. This duplicates a single line of code among two callers, but thins the refspec.h API by one function, and prevents introducing two more in the following commit. Note that we still have a trailing Boolean argument in the function `refspec_item_init()`. The following commit will address this. Signed-off-by: Taylor Blau Acked-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/pull.c | 3 ++- refspec.c | 10 ++-------- refspec.h | 2 -- 3 files changed, 4 insertions(+), 11 deletions(-) (limited to 'builtin/pull.c') diff --git a/builtin/pull.c b/builtin/pull.c index 8bbfcce729..a68a9955de 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -738,7 +738,8 @@ static const char *get_tracking_branch(const char *remote, const char *refspec) const char *spec_src; const char *merge_branch; - refspec_item_init_or_die(&spec, refspec, 1); + if (!refspec_item_init(&spec, refspec, 1)) + die(_("invalid refspec '%s'"), refspec); spec_src = spec.src; if (!*spec_src || !strcmp(spec_src, "HEAD")) spec_src = "HEAD"; diff --git a/refspec.c b/refspec.c index f6be0c54d7..3aeb697505 100644 --- a/refspec.c +++ b/refspec.c @@ -160,13 +160,6 @@ int refspec_item_init(struct refspec_item *item, const char *refspec, int fetch) return parse_refspec(item, refspec, fetch); } -void refspec_item_init_or_die(struct refspec_item *item, const char *refspec, - int fetch) -{ - if (!refspec_item_init(item, refspec, fetch)) - die(_("invalid refspec '%s'"), refspec); -} - void refspec_item_clear(struct refspec_item *item) { FREE_AND_NULL(item->src); @@ -194,7 +187,8 @@ void refspec_append(struct refspec *rs, const char *refspec) { struct refspec_item item; - refspec_item_init_or_die(&item, refspec, rs->fetch); + if (!refspec_item_init(&item, refspec, rs->fetch)) + die(_("invalid refspec '%s'"), refspec); ALLOC_GROW(rs->items, rs->nr + 1, rs->alloc); rs->items[rs->nr] = item; diff --git a/refspec.h b/refspec.h index 7db68e56c8..614f34554e 100644 --- a/refspec.h +++ b/refspec.h @@ -49,8 +49,6 @@ struct refspec { int refspec_item_init(struct refspec_item *item, const char *refspec, int fetch); -void refspec_item_init_or_die(struct refspec_item *item, const char *refspec, - int fetch); void refspec_item_clear(struct refspec_item *item); void refspec_init_fetch(struct refspec *rs); void refspec_init_push(struct refspec *rs); -- cgit v1.3 From 459e54b5497b53f298fe9164112f9bcb33bedb8d Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Tue, 18 Mar 2025 18:50:27 -0400 Subject: refspec: replace `refspec_item_init()` with fetch/push variants For similar reasons as in the previous refactoring of `refspec_init()` into `refspec_init_fetch()` and `refspec_init_push()`, apply the same refactoring to `refspec_item_init()`. Signed-off-by: Taylor Blau Acked-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/fetch.c | 2 +- builtin/pull.c | 2 +- refspec.c | 22 +++++++++++++++++++--- refspec.h | 4 ++-- 4 files changed, 23 insertions(+), 7 deletions(-) (limited to 'builtin/pull.c') diff --git a/builtin/fetch.c b/builtin/fetch.c index 02af505469..9830c09011 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -586,7 +586,7 @@ static struct ref *get_ref_map(struct remote *remote, struct refspec_item tag_refspec; /* also fetch all tags */ - refspec_item_init(&tag_refspec, TAG_REFSPEC, 0); + refspec_item_init_push(&tag_refspec, TAG_REFSPEC); get_fetch_map(remote_refs, &tag_refspec, &tail, 0); refspec_item_clear(&tag_refspec); } else if (tags == TAGS_DEFAULT && *autotags) { diff --git a/builtin/pull.c b/builtin/pull.c index a68a9955de..a1ebc6ad33 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -738,7 +738,7 @@ static const char *get_tracking_branch(const char *remote, const char *refspec) const char *spec_src; const char *merge_branch; - if (!refspec_item_init(&spec, refspec, 1)) + if (!refspec_item_init_fetch(&spec, refspec)) die(_("invalid refspec '%s'"), refspec); spec_src = spec.src; if (!*spec_src || !strcmp(spec_src, "HEAD")) diff --git a/refspec.c b/refspec.c index 3aeb697505..0775358d96 100644 --- a/refspec.c +++ b/refspec.c @@ -153,13 +153,24 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet return 1; } -int refspec_item_init(struct refspec_item *item, const char *refspec, int fetch) +static int refspec_item_init(struct refspec_item *item, const char *refspec, + int fetch) { memset(item, 0, sizeof(*item)); item->raw = xstrdup(refspec); return parse_refspec(item, refspec, fetch); } +int refspec_item_init_fetch(struct refspec_item *item, const char *refspec) +{ + return refspec_item_init(item, refspec, 1); +} + +int refspec_item_init_push(struct refspec_item *item, const char *refspec) +{ + return refspec_item_init(item, refspec, 0); +} + void refspec_item_clear(struct refspec_item *item) { FREE_AND_NULL(item->src); @@ -186,8 +197,13 @@ void refspec_init_push(struct refspec *rs) void refspec_append(struct refspec *rs, const char *refspec) { struct refspec_item item; + int ret; - if (!refspec_item_init(&item, refspec, rs->fetch)) + if (rs->fetch) + ret = refspec_item_init_fetch(&item, refspec); + else + ret = refspec_item_init_push(&item, refspec); + if (!ret) die(_("invalid refspec '%s'"), refspec); ALLOC_GROW(rs->items, rs->nr + 1, rs->alloc); @@ -233,7 +249,7 @@ void refspec_clear(struct refspec *rs) int valid_fetch_refspec(const char *fetch_refspec_str) { struct refspec_item refspec; - int ret = refspec_item_init(&refspec, fetch_refspec_str, 1); + int ret = refspec_item_init_fetch(&refspec, fetch_refspec_str); refspec_item_clear(&refspec); return ret; } diff --git a/refspec.h b/refspec.h index 614f34554e..8b04f9995e 100644 --- a/refspec.h +++ b/refspec.h @@ -47,8 +47,8 @@ struct refspec { unsigned fetch : 1; }; -int refspec_item_init(struct refspec_item *item, const char *refspec, - int fetch); +int refspec_item_init_fetch(struct refspec_item *item, const char *refspec); +int refspec_item_init_push(struct refspec_item *item, const char *refspec); void refspec_item_clear(struct refspec_item *item); void refspec_init_fetch(struct refspec *rs); void refspec_init_push(struct refspec *rs); -- cgit v1.3