[PATCH] add_submodule_odb: initialize alt_odb list earlier

On Wed, Oct 28, 2015 at 02:35:23PM +0100, Mathias L. Baumann wrote:

Yeah, I can reproduce it easily with that. Thanks for providing the repository. It takes a rather convoluted set of conditions to trigger the bug. :)

Here's the fix:

-- >8 --
Subject: add_submodule_odb: initialize alt_odb list earlier

The add_submodule_odb function tries to add a submodule's object store as an "alternate". It needs the existing list to be initialized (from the objects/info/alternates file) for two reasons:

  1. We look for duplicates with the existing alternate
     stores, but obviously this doesn't work if we haven't
     loaded any yet.

  2. We link our new entry into the list by prepending it to
     alt_odb_list. But we do _not_ modify alt_odb_tail.
     This variable starts as NULL, and is a signal to the
     alt_odb code that the list has not yet been
     initialized.

     We then call read_info_alternates on the submodule (to
     recursively load its alternates), which will try to
     append to that tail, assuming it has been initialized.
     This causes us to segfault if it is NULL.

This rarely comes up in practice, because we will have initialized the alt_odb any time we do an object lookup. So you can trigger this only when:

  - you try to access a submodule (e.g., a diff with
    diff.submodule=log)

  - the access happens before any other object has been
    accessed (e.g., because the diff is between the working
    tree and the index)

  - the submodule contains an alternates file (so we try to
    add an entry to the NULL alt_odb_tail)

To fix this, we just need to call prepare_alt_odb at the start of the function (and if we have already initialized, it is a noop).

Note that we can remove the prepare_alt_odb call from the end. It is guaranteed to be a noop, since we will have called it earlier.

Signed-off-by: Jeff King <peff@peff.net>
---
 submodule.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)