path: root/Documentation/technical
diff options
authorJeff King <>2015-05-21 04:45:09 (GMT)
committerJunio C Hamano <>2015-05-21 17:43:50 (GMT)
commitee2499fe38d93fcbf4cbecdf83b32a495b0b12b9 (patch)
tree799aa1513c92b613487bce8c720e5a45be75207e /Documentation/technical
parente41bf352e3280e6990605a18ebbbd40c6f1c0d6d (diff)
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 <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'Documentation/technical')
0 files changed, 0 insertions, 0 deletions