git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] read-cache.c: fix writing "link" index ext with null base oid

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Feb 13, 2019, 12:14 UTC
Message-ID
<87imxnkhqb.fsf@evledraar.gmail.com>
In-Reply-To
<20190213095129.31272-1-pclouds@gmail.com>
On Wed, Feb 13 2019, Nguyễn Thái Ngọc Duy wrote:
Show 35 quoted lines
> Since commit 7db118303a (unpack_trees: fix breakage when o->src_index !=
> o->dst_index - 2018-04-23) and changes in merge code to use separate
> index_state for source and destination, when doing a merge with split
> index activated, we may run into this line in unpack_trees():
>
>     o->result.split_index = init_split_index(&o->result);
>
> This is by itself not wrong. But this split index information is not
> fully populated (and it's only so when move_cache_to_base_index() is
> called, aka force splitting the index, or loading index_state from a
> file). Both "base_oid" and "base" in this case remain null.
>
> So when writing the main index down, we link to this index with null
> oid (default value after init_split_index()), which also means "no split
> index" internally. This triggers an incorrect base index refresh:
>
>     warning: could not freshen shared index '.../sharedindex.0{40}'
>
> This patch makes sure we will not refresh null base_oid (because the
> file is never there). It also makes sure not to write "link" extension
> with null base_oid in the first place (no point having it at
> all). Read code already has protection against null base_oid.
>
> There is also another side fix in remove_split_index() that causes a
> crash when doing "git update-index --no-split-index" when base_oid in
> the index file is null. In this case we will not load
> istate->split_index->base but we dereference it anyway and are rewarded
> with a segfault. This should not happen anymore, but it's still wrong to
> dereference a potential NULL pointer, especially when we do check for
> NULL pointer in the next code.
>
> Reported-by: Luke Diamand <luke@diamand.org>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  v2 added a new test

This round looks good to me. Passes all tests with/without GIT_TEST_SPLIT_INDEX=true

Show 19 quoted lines
>  read-cache.c           |  5 +++--
>  split-index.c          | 34 ++++++++++++++++++----------------
>  t/t1700-split-index.sh | 18 ++++++++++++++++++
>  3 files changed, 39 insertions(+), 18 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index 8f644f68b4..d140b44f8f 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2520,7 +2520,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
>  		return err;
>
>  	/* Write extension data here */
> -	if (!strip_extensions && istate->split_index) {
> +	if (!strip_extensions && istate->split_index &&
> +	    !is_null_oid(&istate->split_index->base_oid)) {
>  		struct strbuf sb = STRBUF_INIT;
>
>  		err = write_link_extension(&sb, istate) < 0 ||

However, it looks like you based this on a pre-2.20.0 version of git. This conflicts with read-cache.c earlier than 3b1d9e045e ("eoie: add End of Index Entry (EOIE) extension", 2018-10-10).

I fixed that manually, and pushed it out to github.com/avar/git.git duy-read-cache-null-split-index-fix, that's what I've tested on top of current "master".

Previous: Nguyễn Thái Ngọc DuyNext: Junio C Hamano
Message 14 of 16 in “could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'”
  1. Luke DiamandFeb 6, 2019
  2. Christian CouderFeb 6, 2019
  3. Duy NguyenFeb 8, 2019
  4. Luke DiamandFeb 8, 2019
  5. Duy NguyenFeb 9, 2019
  6. Luke DiamandFeb 9, 2019
  7. Duy NguyenFeb 9, 2019
  8. unpack-trees.c: fix writing "link" index ext with null base oidNguyễn Thái Ngọc Duy, Feb 9, 2019
  9. Luke DiamandFeb 9, 2019
  10. Elijah NewrenFeb 12, 2019
  11. Junio C HamanoFeb 12, 2019
  12. Ævar Arnfjörð BjarmasonFeb 12, 2019
  13. read-cache.c: fix writing "link" index ext with null base oidNguyễn Thái Ngọc Duy, Feb 13, 2019
  14. Ævar Arnfjörð BjarmasonFeb 13, 2019
  15. Junio C HamanoFeb 8, 2019
  16. Duy NguyenFeb 9, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.