Skip to content

Commit 4689101

Browse files
rjustogitster
authored andcommitted
remote: fix a leak in query_matches_negative_refspec
In c0192df (refspec: add support for negative refspecs, 2020-09-30) query_matches_negative_refspec() was introduced. The function was implemented as a two-loop process, where the former loop accumulates and the latter evaluates. To accumulate, a string_list is used. Within the first loop, there are three cases where a string is added to the string_list. Two of them add strings that do not need to be freed. But in the third case, the string added is returned by match_name_with_pattern(), which needs to be freed. The string_list is initialized with STRING_LIST_INIT_NODUP, i.e. when cleared, the strings added are not freed. Therefore, the string returned by match_name_with_pattern() is not freed, so we have a leak. $ git remote add local . $ git update-ref refs/remotes/local/foo HEAD $ git branch --track bar local/foo Direct leak of 24 byte(s) in 1 object(s) allocated from: ... in xrealloc wrapper.c ... in strbuf_grow strbuf.c ... in strbuf_add strbuf.c ... in match_name_with_pattern remote.c ... in query_matches_negative_refspec remote.c ... in query_refspecs remote.c ... in remote_find_tracking remote.c ... in find_tracked_branch branch.c ... in for_each_remote remote.c ... in setup_tracking branch.c ... in create_branch branch.c ... in cmd_branch builtin/branch.c ... in run_builtin git.c Direct leak of 24 byte(s) in 1 object(s) allocated from: ... in xrealloc wrapper.c ... in strbuf_grow strbuf.c ... in strbuf_add strbuf.c ... in match_name_with_pattern remote.c ... in query_matches_negative_refspec remote.c ... in query_refspecs remote.c ... in remote_find_tracking remote.c ... in check_tracking_branch branch.c ... in for_each_remote remote.c ... in validate_remote_tracking_branch branch.c ... in dwim_branch_start branch.c ... in create_branch branch.c ... in cmd_branch builtin/branch.c ... in run_builtin git.c An interesting point to note is that while string_list_append() is used in the first two cases described, string_list_append_nodup() is used in the third. This seems to indicate an intention to delegate the responsibility for freeing the string, to the string_list. As if the string_list had been initialized with STRING_LIST_INIT_DUP, i.e. the strings are strdup()'d when added (except if the "_nodup" API is used) and freed when cleared. Switching to STRING_LIST_INIT_DUP fixes the leak and probably is what we wanted to do originally. Let's do it. Signed-off-by: Rubén Justo <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 003c1f1 commit 4689101

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

remote.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -890,7 +890,7 @@ static int query_matches_negative_refspec(struct refspec *rs, struct refspec_ite
890890
{
891891
int i, matched_negative = 0;
892892
int find_src = !query->src;
893-
struct string_list reversed = STRING_LIST_INIT_NODUP;
893+
struct string_list reversed = STRING_LIST_INIT_DUP;
894894
const char *needle = find_src ? query->dst : query->src;
895895

896896
/*

0 commit comments

Comments
 (0)