worktree: simplify find_shared_symref() memory ownership model
Storing the worktrees list in a static variable meant that find_shared_symref() had to rebuild the list on each call (which is inefficient when the call site is in a loop), and also that each call invalidated the pointer returned by the previous call (which is confusing). Instead, make it the caller’s responsibility to pass in the worktrees list and manage its lifetime. Signed-off-by: Anders Kaseorg <andersk@mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Anders Kaseorg committed
Dec 1, 2021 at 14:15 UTC
c8dd491fa5b57ecfd9ef33ae5291eb2a9402cd59
6 files changed
+65
-38
branch.c
+8
-6
@@ -357,14 +357,16 @@ void remove_branch_state(struct repository *r, int verbose)
357
358
void die_if_checked_out(const char *branch, int ignore_current_worktree)
359
{
360
+ struct worktree **worktrees = get_worktrees();
361
const struct worktree *wt;
362
362
- wt = find_shared_symref("HEAD", branch);
363
- if (!wt || (ignore_current_worktree && wt->is_current))
364
- return;
365
- skip_prefix(branch, "refs/heads/", &branch);
366
- die(_("'%s' is already checked out at '%s'"),
367
- branch, wt->path);
363
+ wt = find_shared_symref(worktrees, "HEAD", branch);
364
+ if (wt && (!ignore_current_worktree || !wt->is_current)) {
365
+ skip_prefix(branch, "refs/heads/", &branch);
366
+ die(_("'%s' is already checked out at '%s'"), branch, wt->path);
367
+ }
368
+
369
+ free_worktrees(worktrees);
370
}
371
372
int replace_each_worktree_head_symref(const char *oldref, const char *newref,
builtin/branch.c
+6
-1
@@ -192,6 +192,7 @@ static void delete_branch_config(const char *branchname)
192
static int delete_branches(int argc, const char **argv, int force, int kinds,
193
int quiet)
194
{
195
+ struct worktree **worktrees;
196
struct commit *head_rev = NULL;
197
struct object_id oid;
198
char *name = NULL;
@@ -228,6 +229,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
229
if (!head_rev)
230
die(_("Couldn't look up commit object for HEAD"));
231
}
232
+
233
+ worktrees = get_worktrees();
234
+
235
for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
236
char *target = NULL;
237
int flags = 0;
@@ -238,7 +242,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
242
243
if (kinds == FILTER_REFS_BRANCHES) {
244
const struct worktree *wt =
241
- find_shared_symref("HEAD", name);
245
+ find_shared_symref(worktrees, "HEAD", name);
246
if (wt) {
247
error(_("Cannot delete branch '%s' "
248
"checked out at '%s'"),
@@ -299,6 +303,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
303
304
free(name);
305
strbuf_release(&bname);
306
+ free_worktrees(worktrees);
307
308
return ret;
309
}
builtin/notes.c
+5
-1
@@ -861,15 +861,19 @@ static int merge(int argc, const char **argv, const char *prefix)
861
update_ref(msg.buf, default_notes_ref(), &result_oid, NULL, 0,
862
UPDATE_REFS_DIE_ON_ERR);
863
else { /* Merge has unresolved conflicts */
864
+ struct worktree **worktrees;
865
const struct worktree *wt;
866
/* Update .git/NOTES_MERGE_PARTIAL with partial merge result */
867
update_ref(msg.buf, "NOTES_MERGE_PARTIAL", &result_oid, NULL,
868
0, UPDATE_REFS_DIE_ON_ERR);
869
/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
869
- wt = find_shared_symref("NOTES_MERGE_REF", default_notes_ref());
870
+ worktrees = get_worktrees();
871
+ wt = find_shared_symref(worktrees, "NOTES_MERGE_REF",
872
+ default_notes_ref());
873
if (wt)
874
die(_("a notes merge into %s is already in-progress at %s"),
875
default_notes_ref(), wt->path);
876
+ free_worktrees(worktrees);
877
if (create_symref("NOTES_MERGE_REF", default_notes_ref(), NULL))
878
die(_("failed to store link to current notes ref (%s)"),
879
default_notes_ref());
builtin/receive-pack.c
+41
-22
@@ -1486,12 +1486,17 @@ static const char *update(struct command *cmd, struct shallow_info *si)
1486
struct object_id *old_oid = &cmd->old_oid;
1487
struct object_id *new_oid = &cmd->new_oid;
1488
int do_update_worktree = 0;
1489
- const struct worktree *worktree = is_bare_repository() ? NULL : find_shared_symref("HEAD", name);
1489
+ struct worktree **worktrees = get_worktrees();
1490
+ const struct worktree *worktree =
1491
+ is_bare_repository() ?
1492
+ NULL :
1493
+ find_shared_symref(worktrees, "HEAD", name);
1494
1495
/* only refs/... are allowed */
1496
if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
1497
rp_error("refusing to create funny ref '%s' remotely", name);
1494
- return "funny refname";
1498
+ ret = "funny refname";
1499
+ goto out;
1500
}
1501
1502
strbuf_addf(&namespaced_name_buf, "%s%s", get_git_namespace(), name);
@@ -1510,7 +1515,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)
1515
rp_error("refusing to update checked out branch: %s", name);
1516
if (deny_current_branch == DENY_UNCONFIGURED)
1517
refuse_unconfigured_deny();
1513
- return "branch is currently checked out";
1518
+ ret = "branch is currently checked out";
1519
+ goto out;
1520
case DENY_UPDATE_INSTEAD:
1521
/* pass -- let other checks intervene first */
1522
do_update_worktree = 1;
@@ -1521,13 +1527,15 @@ static const char *update(struct command *cmd, struct shallow_info *si)
1527
if (!is_null_oid(new_oid) && !has_object_file(new_oid)) {
1528
error("unpack should have generated %s, "
1529
"but I can't find it!", oid_to_hex(new_oid));
1524
- return "bad pack";
1530
+ ret = "bad pack";
1531
+ goto out;
1532
}
1533
1534
if (!is_null_oid(old_oid) && is_null_oid(new_oid)) {
1535
if (deny_deletes && starts_with(name, "refs/heads/")) {
1536
rp_error("denying ref deletion for %s", name);
1530
- return "deletion prohibited";
1537
+ ret = "deletion prohibited";
1538
+ goto out;
1539
}
1540
1541
if (worktree || (head_name && !strcmp(namespaced_name, head_name))) {
@@ -1543,9 +1551,11 @@ static const char *update(struct command *cmd, struct shallow_info *si)
1551
if (deny_delete_current == DENY_UNCONFIGURED)
1552
refuse_unconfigured_deny_delete_current();
1553
rp_error("refusing to delete the current branch: %s", name);
1546
- return "deletion of the current branch prohibited";
1554
+ ret = "deletion of the current branch prohibited";
1555
+ goto out;
1556
default:
1548
- return "Invalid denyDeleteCurrent setting";
1557
+ ret = "Invalid denyDeleteCurrent setting";
1558
+ goto out;
1559
}
1560
}
1561
}
@@ -1563,25 +1573,30 @@ static const char *update(struct command *cmd, struct shallow_info *si)
1573
old_object->type != OBJ_COMMIT ||
1574
new_object->type != OBJ_COMMIT) {
1575
error("bad sha1 objects for %s", name);
1566
- return "bad ref";
1576
+ ret = "bad ref";
1577
+ goto out;
1578
}
1579
old_commit = (struct commit *)old_object;
1580
new_commit = (struct commit *)new_object;
1581
if (!in_merge_bases(old_commit, new_commit)) {
1582
rp_error("denying non-fast-forward %s"
1583
" (you should pull first)", name);
1573
- return "non-fast-forward";
1584
+ ret = "non-fast-forward";
1585
+ goto out;
1586
}
1587
}
1588
if (run_update_hook(cmd)) {
1589
rp_error("hook declined to update %s", name);
1578
- return "hook declined";
1590
+ ret = "hook declined";
1591
+ goto out;
1592
}
1593
1594
if (do_update_worktree) {
1582
- ret = update_worktree(new_oid->hash, find_shared_symref("HEAD", name));
1595
+ ret = update_worktree(new_oid->hash,
1596
+ find_shared_symref(worktrees, "HEAD",
1597
+ name));
1598
if (ret)
1584
- return ret;
1599
+ goto out;
1600
}
1601
1602
if (is_null_oid(new_oid)) {
@@ -1600,17 +1615,19 @@ static const char *update(struct command *cmd, struct shallow_info *si)
1615
old_oid,
1616
0, "push", &err)) {
1617
rp_error("%s", err.buf);
1603
- strbuf_release(&err);
1604
- return "failed to delete";
1618
+ ret = "failed to delete";
1619
+ } else {
1620
+ ret = NULL; /* good */
1621
}
1622
strbuf_release(&err);
1607
- return NULL; /* good */
1623
}
1624
else {
1625
struct strbuf err = STRBUF_INIT;
1626
if (shallow_update && si->shallow_ref[cmd->index] &&
1612
- update_shallow_ref(cmd, si))
1613
- return "shallow error";
1627
+ update_shallow_ref(cmd, si)) {
1628
+ ret = "shallow error";
1629
+ goto out;
1630
+ }
1631
1632
if (ref_transaction_update(transaction,
1633
namespaced_name,
@@ -1618,14 +1635,16 @@ static const char *update(struct command *cmd, struct shallow_info *si)
1635
0, "push",
1636
&err)) {
1637
rp_error("%s", err.buf);
1621
- strbuf_release(&err);
1622
-
1623
- return "failed to update ref";
1638
+ ret = "failed to update ref";
1639
+ } else {
1640
+ ret = NULL; /* good */
1641
}
1642
strbuf_release(&err);
1626
-
1627
- return NULL; /* good */
1643
}
1644
+
1645
+out:
1646
+ free_worktrees(worktrees);
1647
+ return ret;
1648
}
1649
1650
static void run_update_post_hook(struct command *commands)
worktree.c
+2
-6
@@ -404,17 +404,13 @@ int is_worktree_being_bisected(const struct worktree *wt,
404
* bisect). New commands that do similar things should update this
405
* function as well.
406
*/
407
-const struct worktree *find_shared_symref(const char *symref,
407
+const struct worktree *find_shared_symref(struct worktree **worktrees,
408
+ const char *symref,
409
const char *target)
410
{
411
const struct worktree *existing = NULL;
411
- static struct worktree **worktrees;
412
int i = 0;
413
414
- if (worktrees)
415
- free_worktrees(worktrees);
416
- worktrees = get_worktrees();
417
-
414
for (i = 0; worktrees[i]; i++) {
415
struct worktree *wt = worktrees[i];
416
const char *symref_target;
worktree.h
+3
-2
@@ -143,9 +143,10 @@ void free_worktrees(struct worktree **);
143
/*
144
* Check if a per-worktree symref points to a ref in the main worktree
145
* or any linked worktree, and return the worktree that holds the ref,
146
- * or NULL otherwise. The result may be destroyed by the next call.
146
+ * or NULL otherwise.
147
*/
148
-const struct worktree *find_shared_symref(const char *symref,
148
+const struct worktree *find_shared_symref(struct worktree **worktrees,
149
+ const char *symref,
150
const char *target);
151
152
/*