@matheusbsbs / matheus-bagliano / commits / d36af33081

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>

Jeff King committed Nov 12, 2024 at 03:36 UTC d36af33081cac89a840f4a4cbc4af1ba4aa40392

5 files changed +14 -15

builtin/fetch.c

-1

@@ -463,7 +463,6 @@ static void filter_prefetch_refspec(struct refspec *rs)

463 rs->raw[j - 1] = rs->raw[j];

464 }

465 rs->nr--;

466 - rs->raw_nr--;

466 i--;

467 continue;

468 }

builtin/remote.c

+4 -4

@@ -633,11 +633,11 @@ static int migrate_file(struct remote *remote)

633 git_config_set_multivar(buf.buf, remote->url.v[i], "^$", 0);

634 strbuf_reset(&buf);

635 strbuf_addf(&buf, "remote.%s.push", remote->name);

636 - for (i = 0; i < remote->push.raw_nr; i++)

636 + for (i = 0; i < remote->push.nr; i++)

637 git_config_set_multivar(buf.buf, remote->push.raw[i], "^$", 0);

638 strbuf_reset(&buf);

639 strbuf_addf(&buf, "remote.%s.fetch", remote->name);

640 - for (i = 0; i < remote->fetch.raw_nr; i++)

640 + for (i = 0; i < remote->fetch.nr; i++)

641 git_config_set_multivar(buf.buf, remote->fetch.raw[i], "^$", 0);

642 if (remote->origin == REMOTE_REMOTES)

643 unlink_or_warn(git_path("remotes/%s", remote->name));

@@ -759,12 +759,12 @@ static int mv(int argc, const char **argv, const char *prefix)

759 goto out;

760 }

761

762 - if (oldremote->fetch.raw_nr) {

762 + if (oldremote->fetch.nr) {

763 strbuf_reset(&buf);

764 strbuf_addf(&buf, "remote.%s.fetch", rename.new_name);

765 git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);

766 strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old_name);

767 - for (i = 0; i < oldremote->fetch.raw_nr; i++) {

767 + for (i = 0; i < oldremote->fetch.nr; i++) {

768 char *ptr;

769

770 strbuf_reset(&buf2);

refspec.c

+8 -7

@@ -186,10 +186,12 @@ static void refspec_append_nodup(struct refspec *rs, char *refspec)

186 refspec_item_init_or_die(&item, refspec, rs->fetch);

187

188 ALLOC_GROW(rs->items, rs->nr + 1, rs->alloc);

189 - rs->items[rs->nr++] = item;

189 + rs->items[rs->nr] = item;

190

191 - ALLOC_GROW(rs->raw, rs->raw_nr + 1, rs->raw_alloc);

192 - rs->raw[rs->raw_nr++] = refspec;

191 + ALLOC_GROW(rs->raw, rs->nr + 1, rs->raw_alloc);

192 + rs->raw[rs->nr] = refspec;

193 +

194 + rs->nr++;

195 }

196

197 void refspec_append(struct refspec *rs, const char *refspec)

@@ -217,18 +219,17 @@ void refspec_clear(struct refspec *rs)

219 {

220 int i;

221

220 - for (i = 0; i < rs->nr; i++)

222 + for (i = 0; i < rs->nr; i++) {

223 refspec_item_clear(&rs->items[i]);

224 + free(rs->raw[i]);

225 + }

226

227 FREE_AND_NULL(rs->items);

228 rs->alloc = 0;

229 rs->nr = 0;

230

227 - for (i = 0; i < rs->raw_nr; i++)

228 - free(rs->raw[i]);

231 FREE_AND_NULL(rs->raw);

232 rs->raw_alloc = 0;

231 - rs->raw_nr = 0;

233

234 rs->fetch = 0;

235 }

refspec.h

-1

@@ -45,7 +45,6 @@ struct refspec {

46 char **raw;

47 int raw_alloc;

48 - int raw_nr;

49 int fetch;

50 };

submodule.c

+2 -2

@@ -1174,7 +1174,7 @@ static int push_submodule(const char *path,

1174 if (remote->origin != REMOTE_UNCONFIGURED) {

1175 int i;

1176 strvec_push(&cp.args, remote->name);

1177 - for (i = 0; i < rs->raw_nr; i++)

1177 + for (i = 0; i < rs->nr; i++)

1178 strvec_push(&cp.args, rs->raw[i]);

1179 }

1180

@@ -1209,7 +1209,7 @@ static void submodule_push_check(const char *path, const char *head,

1209 strvec_push(&cp.args, head);

1210 strvec_push(&cp.args, remote->name);

1211

1212 - for (i = 0; i < rs->raw_nr; i++)

1212 + for (i = 0; i < rs->nr; i++)

1213 strvec_push(&cp.args, rs->raw[i]);

1214

1215 prepare_submodule_repo_env(&cp.env);