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

> 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?