[PATCH v2 0/2] fix memory leaks in transport_push()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux