[PATCH] clone: plug a miniscule leak

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

 



The remote_name variable is first assigned a copy of the value of
the "clone.defaultremotename" configuration variable and then by the
value of the "--origin" command line option.  The former is prepared
to see multiple instances of the variable by freeing the current
value of the variable before a copy of the newly discovered value
gets assigned to it.  The latter blindly assigned a copy of the new
value to it, thereby leaking the value read from the configuration.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---

    This was discovered by a recently removed bogus coccinelle
    rewrite rule---if you apply an incorrect change suggested by the
    bogus rewrite rule to unconditionally assign to remote_name a
    copy of option_origin, or NULL, the value read from the
    configuration would be lost and never be used, thereby breaking
    a test to ensure the configuration is used, instead of the
    default remote nickname "origin".

    Perhaps a Coccinelle rule like this might have caught similar
    leaks:

	@@
	expression E;
	expression V;
	@@
	- if (E)
	-   V = xstrdup(E);
	+ if (E) {
	+   free(V);
	+   V = xstrdup(E);
	+ }

    The fact that the result of xstrdup() is assigned to V is that V
    is meant to hold a pointer to an allocated piece of memory.

    With the preimage of the above semantic patch, it is reasonable
    to expect that V may be initialized to NULL or may be holding a
    pointer to a piece of allocated memory when the control reaches
    here, because otherwise, V will be either need to be freed (when
    E was not NULL, in which case we assigned the result of
    xstrdup() to it) or V has garbage that cannot be freed later.
   
 builtin/clone.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git c/builtin/clone.c w/builtin/clone.c
index 5231656379..194d50f75f 100644
--- c/builtin/clone.c
+++ w/builtin/clone.c
@@ -1106,8 +1106,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	 * apply the remote name provided by --origin only after this second
 	 * call to git_config, to ensure it overrides all config-based values.
 	 */
-	if (option_origin != NULL)
+	if (option_origin != NULL) {
+		free(remote_name);
 		remote_name = xstrdup(option_origin);
+	}
 
 	if (remote_name == NULL)
 		remote_name = xstrdup("origin");



[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