From: Jon Smirl <jonsmirl@gmail.com>
Date: Sat, 17 Jun 2006 04:15:57 GMT
Subject: Re: parsecvs and unnamed branches
Message-ID: <9e4733910606162115g2165212bgf32a2e328cce751a@mail.gmail.com>
In-Reply-To: <1150517336.9144.8.camel@dv>

On 6/17/06, Pavel Roskin <proski@gnu.org> wrote:
> On Fri, 2006-06-16 at 23:31 -0400, Jon Smirl wrote:
>
> > Parsecvs was compiled '-O2 -g' why didn't it decode the addresses to symbols?
>
> Sorry, I was too quick to put blame on you.  Maybe glibc can only list
> its own symbols.
>
> I could reproduce the problem trivially with a single file, and here's
> what Valgrind says:
>
> ==11154== Invalid free() / delete / delete[]
> ==11154==    at 0x4905423: free (vg_replace_malloc.c:233)
> ==11154==    by 0x40C136: git_pack_directory (git.c:620)
> ==11154==    by 0x40C1B4: git_rev_list_pack (git.c:639)
> ==11154==    by 0x4067DA: main (parsecvs.c:785)
>
>
> git_pack_directory() tries to free() the result of
> git_system_to_string(), which is in turn a result of atom().  My
> understanding is that atoms should not be freed.  They are not freed in
> other cases.
>
> Patch:
>
> diff --git a/README b/README
> diff --git a/git.c b/git.c
> index 33b29c7..7312568 100644
> --- a/git.c
> +++ b/git.c
> @@ -617,7 +617,6 @@ git_pack_directory (void)
>         }
>         free (objects_dir);
>         pack_dir = git_format_command ("%s/objects/pack", git_dir);
> -        free (git_dir);
>         if (!pack_dir)
>             return NULL;
>         if (access (pack_dir, F_OK) == -1 &&

I had already caught that one, the fix was a few mails back.
git_dir is an atom and shouldn't be freed with free.

After five hours I hit this:
fprintf (stderr, "Error: branch cycle\n");

static rev_ref *
rev_ref_tsort (rev_ref *refs, rev_list *head)
{
    rev_ref *done = NULL;
    rev_ref **done_tail = &done;
    rev_ref *r, **prev;

//    fprintf (stderr, "Tsort refs:\n");
    while (refs) {
        for (prev = &refs; (r = *prev); prev = &(*prev)->next) {
            if (rev_ref_is_ready (r->name, head, done)) {
                break;
            }
        }
        if (!r) {
            fprintf (stderr, "Error: branch cycle\n");
>> hit this test
            return NULL;
        }
        *prev = r->next;
        *done_tail = r;
//      fprintf (stderr, "\t%s\n", r->name);
        r->next = NULL;
        done_tail = &r->next;
    }
    return done;
}

which returned null up to here

    if (rev_mode == ExecuteGit && pack_objcount && autopack)
        git_rev_list_pack (pack_start, strip);
    load_status_next ();
    rl = rev_list_merge (head);
>> null to here
    if (rl) {
        switch (rev_mode) {
        case ExecuteGraph:
            dump_rev_graph (rl, NULL);
            break;
        case ExecuteSplits:
            dump_splits (rl);
            break;
        case ExecuteGit:
            git_rev_list_commit (rl, strip);
            break;
        }
    }
    if (rl)
        rev_list_free (rl, 0);
    while (head) {
        rl = head;
        head = head->next;
        rev_list_free (rl, 1);
>> tries to free the list, but the list is a loop.
>> after it wraps it will mangle memory

    }
    discard_atoms ();
    rev_free_dirs ();
    rev_commit_cleanup ();
    git_free_author_map ();
    return err;

>>But the real problem is why does it think the branches are in a loop?

-- 
Jon Smirl
jonsmirl@gmail.com