summaryrefslogtreecommitdiff
path: root/remote.c
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2015-05-21 00:45:09 -0400
committerJunio C Hamano <gitster@pobox.com>2015-05-21 10:43:50 -0700
commitee2499fe38d93fcbf4cbecdf83b32a495b0b12b9 (patch)
tree799aa1513c92b613487bce8c720e5a45be75207e /remote.c
parente41bf352e3280e6990605a18ebbbd40c6f1c0d6d (diff)
downloadgit-ee2499fe38d93fcbf4cbecdf83b32a495b0b12b9.tar.gz
remote.c: refactor setup of branch->merge list
When we call branch_get() to lookup or create a "struct branch", we make sure the "merge" field is filled in so that callers can access it. But the conditions under which we do so are a little confusing, and can lead to two funny situations: 1. If there's no branch.*.remote config, we cannot provide branch->merge (because it is really just an application of branch.*.merge to our remote's refspecs). But branch->merge_nr may be non-zero, leading callers to be believe they can access branch->merge (e.g., in branch_merge_matches and elsewhere). It doesn't look like this can cause a segfault in practice, as most code paths dealing with merge config will bail early if there is no remote defined. But it's a bit of a dangerous construct. We can fix this by setting merge_nr to "0" explicitly when we realize that we have no merge config. Note that merge_nr also counts the "merge_name" fields (which we _do_ have; that's how merge_nr got incremented), so we will "lose" access to them, in the sense that we forget how many we had. But no callers actually care; we use merge_name only while iteratively reading the config, and then convert it to the final "merge" form the first time somebody calls branch_get(). 2. We set up the "merge" field every time branch_get is called, even if it has already been done. This leaks memory. It's not a big deal in practice, since most code paths will access only one branch, or perhaps each branch only one time. But if you want to be pathological, you can leak arbitrary memory with: yes @{upstream} | head -1000 | git rev-list --stdin We can fix this by skipping setup when branch->merge is already non-NULL. In addition to those two fixes, this patch pushes the "do we need to setup merge?" logic down into set_merge, where it is a bit easier to follow. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'remote.c')
-rw-r--r--remote.c19
1 files changed, 15 insertions, 4 deletions
diff --git a/remote.c b/remote.c
index bec8b311b0..ac17e66c09 100644
--- a/remote.c
+++ b/remote.c
@@ -1636,6 +1636,19 @@ static void set_merge(struct branch *ret)
unsigned char sha1[20];
int i;
+ if (!ret)
+ return; /* no branch */
+ if (ret->merge)
+ return; /* already run */
+ if (!ret->remote_name || !ret->merge_nr) {
+ /*
+ * no merge config; let's make sure we don't confuse callers
+ * with a non-zero merge_nr but a NULL merge
+ */
+ ret->merge_nr = 0;
+ return;
+ }
+
ret->merge = xcalloc(ret->merge_nr, sizeof(*ret->merge));
for (i = 0; i < ret->merge_nr; i++) {
ret->merge[i] = xcalloc(1, sizeof(**ret->merge));
@@ -1660,11 +1673,9 @@ struct branch *branch_get(const char *name)
ret = current_branch;
else
ret = make_branch(name, 0);
- if (ret && ret->remote_name) {
+ if (ret && ret->remote_name)
ret->remote = remote_get(ret->remote_name);
- if (ret->merge_nr)
- set_merge(ret);
- }
+ set_merge(ret);
return ret;
}