Re: excerpts from tomorrow's "What's cooking" draft

From: Sverre Rabbelier <srabbelier@gmail.com>
Date: Nov 15, 2009, 02:07 UTC
Message-ID: <fabb9a1e0911141807q4be6015bjda4805496be6dbb3@mail.gmail.com>
In-Reply-To: <alpine.LNX.2.00.0911111259520.14365@iabervon.org>

Heya,

On Wed, Nov 11, 2009 at 19:45, Daniel Barkalow <barkalow@iabervon.org> wrote:

> Okay, finally got to it just now. This is entirely untested (because I
> don't have anything that uses it), but it compiles and should be correct.
> It would be nice to get a clean bill of health on it from valgrind.

After a lot of manual comparison of valgrind traces (comparing cloning a local git repo and a local hg repo), I have found the following before applying your patch (I removed the ==proc== and "by 0xDEADBEEF" noise):

  20 bytes in 1 blocks are definitely lost in loss record 24 of 95
    at : calloc (vg_replace_malloc.c:418)
    by : xcalloc (wrapper.c:75)
    by : get_importer (transport-helper.c:132)
    by : fetch_with_import (transport-helper.c:150)
    by : fetch (transport-helper.c:198)
    by : transport_fetch_refs (transport.c:977)
    by : cmd_clone (builtin-clone.c:538)
    by : run_builtin (git.c:251)
    by : handle_internal_command (git.c:396)
    by : run_argv (git.c:438)
    by : main (git.c:509)

  60 bytes in 1 blocks are definitely lost in loss record 43 of 95
    at : realloc (vg_replace_malloc.c:476)
    by : xrealloc (wrapper.c:59)
    by : strbuf_grow (strbuf.c:61)
    by : strbuf_getwholeline (strbuf.c:335)
    by : strbuf_getline (strbuf.c:349)
    by : get_helper (transport-helper.c:52)
    by : get_refs_list (transport-helper.c:228)
    by : transport_get_remote_refs (transport.c:943)
    by : cmd_clone (builtin-clone.c:535)
    by : run_builtin (git.c:251)
    by : handle_internal_command (git.c:396)
    by : run_argv (git.c:438)

  65 bytes in 1 blocks are definitely lost in loss record 47 of 95
    at : malloc (vg_replace_malloc.c:195)
    by : realloc (vg_replace_malloc.c:476)
    by : xrealloc (wrapper.c:59)
    by : strbuf_grow (strbuf.c:61)
    by : strbuf_addf (strbuf.c:199)
    by : get_helper (transport-helper.c:69)
    by : get_refs_list (transport-helper.c:228)
    by : transport_get_remote_refs (transport.c:943)
    by : cmd_clone (builtin-clone.c:535)
    by : run_builtin (git.c:251)
    by : handle_internal_command (git.c:396)
    by : run_argv (git.c:438)

  65 bytes in 1 blocks are definitely lost in loss record 50 of 95
    at : malloc (vg_replace_malloc.c:195)
    by : realloc (vg_replace_malloc.c:476)
    by : xrealloc (wrapper.c:59)
    by : strbuf_grow (strbuf.c:61)
    by : strbuf_addf (strbuf.c:199)
    by : fetch_with_import (transport-helper.c:158)
    by : fetch (transport-helper.c:198)
    by : transport_fetch_refs (transport.c:977)
    by : cmd_clone (builtin-clone.c:538)
    by : run_builtin (git.c:251)
    by : handle_internal_command (git.c:396)
    by : run_argv (git.c:438)

  123 (96 direct, 27 indirect) bytes in 1 blocks are definitely lost
in loss record 71 of 95
    at : malloc (vg_replace_malloc.c:195)
    by : realloc (vg_replace_malloc.c:476)
    by : xrealloc (wrapper.c:59)
    by : get_helper (transport-helper.c:62)
    by : get_refs_list (transport-helper.c:228)
    by : transport_get_remote_refs (transport.c:943)
    by : cmd_clone (builtin-clone.c:535)
    by : run_builtin (git.c:251)
    by : handle_internal_command (git.c:396)
    by : run_argv (git.c:438)
    by : main (git.c:509)

Annotated with the relevant source and my comments:
    by : get_importer (transport-helper.c:132)
	fastimport->argv = xcalloc(5, sizeof(*fastimport->argv));

You never free fastimport->argv.

    by : get_helper (transport-helper.c:52)
		if (strbuf_getline(&buf, file, '\n') == EOF)

You never strbuf_release(&buf).

    by : get_helper (transport-helper.c:69)
			strbuf_addf(&gitdir, "gitdir %s\n", get_git_dir());

I do strbuf_reset(&gitdir), which should of course be strbuf_release(&gitdir), oops.

    by : fetch_with_import (transport-helper.c:158)
		strbuf_addf(&buf, "import %s\n", posn->name);

Same here, you never strbuf_release(&buf).

    by : get_helper (transport-helper.c:62)
			ALLOC_GROW(refspecs,
				   refspec_nr + 1,
				   refspec_alloc);

This would of course be fixed by your patch.

However, applying your patch causes: error: Trying to write ref refs/heads/tip with nonexistant object 0000000000000000000000000000000000000000 fatal: Cannot update the ref 'HEAD'.

If I comment out both new lines in disonnect_helper like so fixes that:
	//free_refspec(data->refspec_nr, data->refspecs);
	//data->refspecs = NULL;

Commenting out only one of the two makes the error return, any idea's?

-- 
Cheers,

Sverre Rabbelier