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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 1, 2009, 16:23 UTC
Message-ID: <7v3acs1gyf.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20090401161254.GD2237@lilem.mirepesht>

Ali Gholami Rudi <ali@rudi.ir> writes:

Show 17 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> On Wed, 1 Apr 2009, Ali Gholami Rudi wrote:
>> 
>> > 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.