Re: [PATCH] builtin-clone.c: fix memory leak in cmd_clone()

Ali Gholami Rudi [off-list ref] writes:

Johannes Schindelin [off-list ref] wrote:

quoted

On Wed, 1 Apr 2009, Ali Gholami Rudi wrote:

quoted

With this patch, cmd_clone() safely frees its xstrdup()-allocated
memory.  Also junk_work_tree and junk_git_dir (used in remove_junk()
which is called asynchronously) were changed to use static arrays rather
than sharing the memory allocated in cmd_clone().

If you want to go down that route, you will have a long way to go: the 
assumption is pretty much in every cmd_() and main() function that 
singletons will be free()d automatically when the process ends.

Well... I saw strbuf_release() calls in the end of cmd_clone() and had a
quick look at a few other cmd_*() functions; it seems most of them (?)
try to free their memory.  I thought it might make sense to do that for
cmd_clone().  But you're right; they will be freed eventually.  (It
seems like a minor leak which is respected only some of the times :-) )

Yup, I'll queue (I won't have time today to work on git it seems) the
other two patches from you, but I was going to drop this one---unless your
plan was to make cmd_clone() callable more than once in order to use it in
say a C rewrite of git submodule or something like that.

Hi,

Junio C Hamano [off-list ref] wrote:

Yup, I'll queue (I won't have time today to work on git it seems) the

Thanks.

other two patches from you, but I was going to drop this one---unless your
plan was to make cmd_clone() callable more than once in order to use it in
say a C rewrite of git submodule or something like that.

The only problem in builtin-clone.c seems to be remove_junk() which is
called from a signal handler or atexit().  cmd_clone() can be changed to
register this function only once.

remove_junk() uses junk_* global variables which are overwritten in each
cmd_clone() call.  Since no concurrent cmd_clone() is allowed (?) and we
only care about the last one, this does not seem to be an issue.

I'll probably send a new patch tomorrow.

Regards,
Ali