Fix memory leaks in transport_push(), where remote_refs and local_refs are never freed. While at it, remove the unnecessary indenting and make the code hopefully more readable. Changes since v1: * Slit into series of two patches. The first one just changes indenting in transport_push(). Second one adds the fix for the local_refs and remote_refs memory leaks. * The resulting trees are the same, there is no code change. Frantisek Hrbata (2): transport: remove unnecessary indenting in transport_push() transport: free local and remote refs in transport_push() transport.c | 254 +++++++++++++++++++++++++++------------------------- 1 file changed, 130 insertions(+), 124 deletions(-) Range-diff against v1: 1: fb8b90d247 ! 1: daf042cd2e transport: free local and remote refs in transport_push() @@ Metadata Author: Frantisek Hrbata <frantisek@xxxxxxxxxx> ## Commit message ## - transport: free local and remote refs in transport_push() + transport: remove unnecessary indenting in transport_push() - Fix memory leaks in transport_push(), where remote_refs and local_refs - are never freed. While at it, remove the unnecessary indenting and make - the code hopefully more readable. - - 116 bytes in 1 blocks are definitely lost in loss record 56 of 103 - at 0x484486F: malloc (vg_replace_malloc.c:381) - by 0x4938D7E: strdup (strdup.c:42) - by 0x628418: xstrdup (wrapper.c:39) - by 0x4FD454: process_capabilities (connect.c:232) - by 0x4FD454: get_remote_heads (connect.c:354) - by 0x610A38: handshake (transport.c:333) - by 0x612B02: transport_push (transport.c:1302) - by 0x4803D6: push_with_options (push.c:357) - by 0x4811D6: do_push (push.c:414) - by 0x4811D6: cmd_push (push.c:650) - by 0x405210: run_builtin (git.c:465) - by 0x405210: handle_builtin (git.c:719) - by 0x406363: run_argv (git.c:786) - by 0x406363: cmd_main (git.c:917) - by 0x404F17: main (common-main.c:56) - - 5,912 (388 direct, 5,524 indirect) bytes in 2 blocks are definitely lost in loss record 98 of 103 - at 0x4849464: calloc (vg_replace_malloc.c:1328) - by 0x628705: xcalloc (wrapper.c:150) - by 0x5C216D: alloc_ref_with_prefix (remote.c:975) - by 0x5C232A: alloc_ref (remote.c:983) - by 0x5C232A: one_local_ref (remote.c:2299) - by 0x5C232A: one_local_ref (remote.c:2289) - by 0x5BDB03: do_for_each_repo_ref_iterator (iterator.c:418) - by 0x5B4C4F: do_for_each_ref (refs.c:1486) - by 0x5B4C4F: refs_for_each_ref (refs.c:1492) - by 0x5B4C4F: for_each_ref (refs.c:1497) - by 0x5C6ADF: get_local_heads (remote.c:2310) - by 0x612A85: transport_push (transport.c:1286) - by 0x4803D6: push_with_options (push.c:357) - by 0x4811D6: do_push (push.c:414) - by 0x4811D6: cmd_push (push.c:650) - by 0x405210: run_builtin (git.c:465) - by 0x405210: handle_builtin (git.c:719) - by 0x406363: run_argv (git.c:786) - by 0x406363: cmd_main (git.c:917) + Remove the big indented block for push_refs() check in transport vtable + and let's just return error immediately. Hopefully this makes the code + more readable. Signed-off-by: Frantisek Hrbata <frantisek@xxxxxxxxxx> @@ transport.c: int transport_push(struct repository *r, struct refspec *rs, int flags, unsigned int *reject_reasons) { -+ struct ref *remote_refs = NULL; -+ struct ref *local_refs = NULL; ++ struct ref *remote_refs; ++ struct ref *local_refs; + int match_flags = MATCH_REFS_NONE; + int verbose = (transport->verbose > 0); + int quiet = (transport->verbose < 0); @@ transport.c: int transport_push(struct repository *r, - - if (check_push_refs(local_refs, rs) < 0) - return -1; -- ++ if (!transport->vtable->push_refs) ++ return 1; + - refspec_ref_prefixes(rs, &transport_options.ref_prefixes); -- ++ local_refs = get_local_heads(); + - trace2_region_enter("transport_push", "get_refs_list", r); - remote_refs = transport->vtable->get_refs_list(transport, 1, - &transport_options); - trace2_region_leave("transport_push", "get_refs_list", r); -- ++ if (check_push_refs(local_refs, rs) < 0) ++ return -1; + - transport_ls_refs_options_release(&transport_options); -- ++ refspec_ref_prefixes(rs, &transport_options.ref_prefixes); + - if (flags & TRANSPORT_PUSH_ALL) - match_flags |= MATCH_REFS_ALL; - if (flags & TRANSPORT_PUSH_MIRROR) @@ transport.c: int transport_push(struct repository *r, - match_flags |= MATCH_REFS_PRUNE; - if (flags & TRANSPORT_PUSH_FOLLOW_TAGS) - match_flags |= MATCH_REFS_FOLLOW_TAGS; -- ++ trace2_region_enter("transport_push", "get_refs_list", r); ++ remote_refs = transport->vtable->get_refs_list(transport, 1, ++ &transport_options); ++ trace2_region_leave("transport_push", "get_refs_list", r); + - if (match_push_refs(local_refs, &remote_refs, rs, match_flags)) - return -1; -- ++ transport_ls_refs_options_release(&transport_options); + - if (transport->smart_options && - transport->smart_options->cas && - !is_empty_cas(transport->smart_options->cas)) - apply_push_cas(transport->smart_options->cas, - transport->remote, remote_refs); -- ++ if (flags & TRANSPORT_PUSH_ALL) ++ match_flags |= MATCH_REFS_ALL; ++ if (flags & TRANSPORT_PUSH_MIRROR) ++ match_flags |= MATCH_REFS_MIRROR; ++ if (flags & TRANSPORT_PUSH_PRUNE) ++ match_flags |= MATCH_REFS_PRUNE; ++ if (flags & TRANSPORT_PUSH_FOLLOW_TAGS) ++ match_flags |= MATCH_REFS_FOLLOW_TAGS; + - set_ref_status_for_push(remote_refs, - flags & TRANSPORT_PUSH_MIRROR, - flags & TRANSPORT_PUSH_FORCE); -- ++ if (match_push_refs(local_refs, &remote_refs, rs, match_flags)) ++ return -1; + - if (!(flags & TRANSPORT_PUSH_NO_HOOK)) - if (run_pre_push_hook(transport, remote_refs)) - return -1; -+ if (!transport->vtable->push_refs) -+ return 1; ++ if (transport->smart_options && ++ transport->smart_options->cas && ++ !is_empty_cas(transport->smart_options->cas)) ++ apply_push_cas(transport->smart_options->cas, ++ transport->remote, remote_refs); - if ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND | - TRANSPORT_RECURSE_SUBMODULES_ONLY)) && @@ transport.c: int transport_push(struct repository *r, - trace2_region_leave("transport_push", "push_submodules", r); - die(_("failed to push all needed submodules")); - } -+ ret = -1; -+ local_refs = get_local_heads(); -+ -+ if (check_push_refs(local_refs, rs) < 0) -+ goto done; -+ -+ refspec_ref_prefixes(rs, &transport_options.ref_prefixes); -+ -+ trace2_region_enter("transport_push", "get_refs_list", r); -+ remote_refs = transport->vtable->get_refs_list(transport, 1, -+ &transport_options); -+ trace2_region_leave("transport_push", "get_refs_list", r); -+ -+ transport_ls_refs_options_release(&transport_options); -+ -+ if (flags & TRANSPORT_PUSH_ALL) -+ match_flags |= MATCH_REFS_ALL; -+ if (flags & TRANSPORT_PUSH_MIRROR) -+ match_flags |= MATCH_REFS_MIRROR; -+ if (flags & TRANSPORT_PUSH_PRUNE) -+ match_flags |= MATCH_REFS_PRUNE; -+ if (flags & TRANSPORT_PUSH_FOLLOW_TAGS) -+ match_flags |= MATCH_REFS_FOLLOW_TAGS; -+ -+ if (match_push_refs(local_refs, &remote_refs, rs, match_flags)) -+ goto done; -+ -+ if (transport->smart_options && -+ transport->smart_options->cas && -+ !is_empty_cas(transport->smart_options->cas)) -+ apply_push_cas(transport->smart_options->cas, -+ transport->remote, remote_refs); -+ + set_ref_status_for_push(remote_refs, + flags & TRANSPORT_PUSH_MIRROR, + flags & TRANSPORT_PUSH_FORCE); + + if (!(flags & TRANSPORT_PUSH_NO_HOOK)) + if (run_pre_push_hook(transport, remote_refs)) -+ goto done; ++ return -1; + + if ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND | + TRANSPORT_RECURSE_SUBMODULES_ONLY)) && @@ transport.c: int transport_push(struct repository *r, - return ret; - } - return 1; -+done: -+ free_refs(local_refs); -+ free_refs(remote_refs); + return ret; } -: ---------- > 2: 9d25176b92 transport: free local and remote refs in transport_push() base-commit: e54793a95afeea1e10de1e5ad7eab914e7416250 -- 2.35.1