refspec: drop separate raw_nr count

A refspec struct contains zero or more refspec_item structs, along with
matching "raw" strings. The items and raw strings are kept in separate
arrays, but those arrays will always have the same length (because we
write them only via refspec_append_nodup(), which grows both). This can
lead to bugs when manipulating the array, since the arrays and lengths
must be modified in lockstep. For example, the bug fixed in the previous
commit, which forgot to decrement raw_nr.

So let's get rid of "raw_nr" and have only "nr", making this kind of bug
impossible (and also making it clear that the two are always matched,
something that existing code already assumed but was not guaranteed by
the interface).

Even though we'd expect "alloc" and "raw_alloc" to likewise move in
lockstep, we still need to keep separate counts there if we want to
continue to use ALLOC_GROW() for both.

Conceptually this would all be simpler if refspec_item just held onto
its own raw string, and we had a single array. But there are callers
which use refspec_item outside of "struct refspec" (and so don't hold on
to a matching "raw" string at all), which we'd possibly need to adjust.
So let's not worry about refactoring that for now, and just get rid of
the redundant count variable. That is the first step on the road to
combining them anyway.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1c59e1c256c151f905053c2e89ba6cceb838171f..80a64d0d269ed60708d7533977f9ba5c51a4b084 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -463,7 +463,6 @@ static void filter_prefetch_refspec(struct refspec *rs)
                                rs->raw[j - 1] = rs->raw[j];
                        }
                        rs->nr--;
-                       rs->raw_nr--;
                        i--;
                        continue;
                }

diff --git a/builtin/remote.c b/builtin/remote.c
index 76670ddd8b44e5b118c76522613c445ad92c5364..875d6c3badcf726824ef4979f98d1350c5bcaed4 100644 (file)
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -633,11 +633,11 @@ static int migrate_file(struct remote *remote)
                git_config_set_multivar(buf.buf, remote->url.v[i], "^$", 0);
        strbuf_reset(&buf);
        strbuf_addf(&buf, "remote.%s.push", remote->name);
-       for (i = 0; i < remote->push.raw_nr; i++)
+       for (i = 0; i < remote->push.nr; i++)
                git_config_set_multivar(buf.buf, remote->push.raw[i], "^$", 0);
        strbuf_reset(&buf);
        strbuf_addf(&buf, "remote.%s.fetch", remote->name);
-       for (i = 0; i < remote->fetch.raw_nr; i++)
+       for (i = 0; i < remote->fetch.nr; i++)
                git_config_set_multivar(buf.buf, remote->fetch.raw[i], "^$", 0);
        if (remote->origin == REMOTE_REMOTES)
                unlink_or_warn(git_path("remotes/%s", remote->name));
@@ -759,12 +759,12 @@ static int mv(int argc, const char **argv, const char *prefix)
                goto out;
        }
 
-       if (oldremote->fetch.raw_nr) {
+       if (oldremote->fetch.nr) {
                strbuf_reset(&buf);
                strbuf_addf(&buf, "remote.%s.fetch", rename.new_name);
                git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);
                strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old_name);
-               for (i = 0; i < oldremote->fetch.raw_nr; i++) {
+               for (i = 0; i < oldremote->fetch.nr; i++) {
                        char *ptr;
 
                        strbuf_reset(&buf2);

diff --git a/refspec.c b/refspec.c
index c3cf003443dd362390185fe471ed889e66b8dd1a..8e8ee8542dd89a02128457da7381c8ae377b5fef 100644 (file)
--- a/refspec.c
+++ b/refspec.c
@@ -186,10 +186,12 @@ static void refspec_append_nodup(struct refspec *rs, char *refspec)
        refspec_item_init_or_die(&item, refspec, rs->fetch);
 
        ALLOC_GROW(rs->items, rs->nr + 1, rs->alloc);
-       rs->items[rs->nr++] = item;
+       rs->items[rs->nr] = item;
 
-       ALLOC_GROW(rs->raw, rs->raw_nr + 1, rs->raw_alloc);
-       rs->raw[rs->raw_nr++] = refspec;
+       ALLOC_GROW(rs->raw, rs->nr + 1, rs->raw_alloc);
+       rs->raw[rs->nr] = refspec;
+
+       rs->nr++;
 }
 
 void refspec_append(struct refspec *rs, const char *refspec)
@@ -217,18 +219,17 @@ void refspec_clear(struct refspec *rs)
 {
        int i;
 
-       for (i = 0; i < rs->nr; i++)
+       for (i = 0; i < rs->nr; i++) {
                refspec_item_clear(&rs->items[i]);
+               free(rs->raw[i]);
+       }
 
        FREE_AND_NULL(rs->items);
        rs->alloc = 0;
        rs->nr = 0;
 
-       for (i = 0; i < rs->raw_nr; i++)
-               free(rs->raw[i]);
        FREE_AND_NULL(rs->raw);
        rs->raw_alloc = 0;
-       rs->raw_nr = 0;
 
        rs->fetch = 0;
 }

diff --git a/refspec.h b/refspec.h
index 3760fdaf2bb1833b0314adf8e66265150c74ccce..0461c9def6261c8e97df64502cdeb58f399e9a33 100644 (file)
--- a/refspec.h
+++ b/refspec.h
@@ -45,7 +45,6 @@ struct refspec {
 
        char **raw;
        int raw_alloc;
-       int raw_nr;
 
        int fetch;
 };

diff --git a/submodule.c b/submodule.c
index 74d5766f07c31149047afe190add31473182b3ef..307f73fb5b13b76c74dc64b72f93aa639fd81090 100644 (file)
--- a/submodule.c
+++ b/submodule.c
@@ -1174,7 +1174,7 @@ static int push_submodule(const char *path,
                if (remote->origin != REMOTE_UNCONFIGURED) {
                        int i;
                        strvec_push(&cp.args, remote->name);
-                       for (i = 0; i < rs->raw_nr; i++)
+                       for (i = 0; i < rs->nr; i++)
                                strvec_push(&cp.args, rs->raw[i]);
                }
 
@@ -1209,7 +1209,7 @@ static void submodule_push_check(const char *path, const char *head,
        strvec_push(&cp.args, head);
        strvec_push(&cp.args, remote->name);
 
-       for (i = 0; i < rs->raw_nr; i++)
+       for (i = 0; i < rs->nr; i++)
                strvec_push(&cp.args, rs->raw[i]);
 
        prepare_submodule_repo_env(&cp.env);