{"thread":{"id":"50409","subject":"could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'","startedAt":"2019-02-06T10:25:39Z","lastAt":"2019-02-13T12:15:04Z","messageCount":16,"participants":["Luke Diamand","Christian Couder","Duy Nguyen","Junio C Hamano","Nguyễn Thái Ngọc Duy","Elijah Newren","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"368572","messageId":"CAE5ih79DuU5Wrk4p+wpQX3j6bgyA2_ZgXYJX-tGvZpYhWazWqA@mail.gmail.com","threadId":"50409","inReplyTo":null,"subject":"could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2019-02-06T10:25:25Z","receivedAt":"2019-02-06T10:25:39Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"I've recently started seeing a lot of this message when doing a rebase:\n\n warning: could not freshen shared index\n'/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.0000000000000000000000000000000000000000'\n\n(There's a repo called dev_full, and I've got a worktree where I'm\nworking on my 3rd attempt to make it work with gcc8).\n\nThat file doesn't actually exist but there are a bunch of\nsharedindex.XXX files in there with more convincing looking names.\n\n2.20.1.611.gfbb209baf1\n"},{"id":"368587","messageId":"CAP8UFD3K47mfp_7gt2J19TZCXH4u4kf5D8McXd9ioguWb6umZQ@mail.gmail.com","threadId":"50409","inReplyTo":"CAE5ih79DuU5Wrk4p+wpQX3j6bgyA2_ZgXYJX-tGvZpYhWazWqA@mail.gmail.com","subject":"Re: could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-02-06T13:17:23Z","receivedAt":"2019-02-06T13:17:38Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Feb 6, 2019 at 11:25 AM Luke Diamand wrote:\n>\n> I've recently started seeing a lot of this message when doing a rebase:\n>\n> warning: could not freshen shared index\n> '/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.0000000000000000000000000000000000000000'\n\nThanks for the report!\n\nHave you tried to run the test suite with GIT_TEST_SPLIT_INDEX set to\n\"true\" on your machine/environment?\nAlso does the rebase fail or otherwise misbehave?\n"},{"id":"368811","messageId":"20190208100221.GA22283@ash","threadId":"50409","inReplyTo":"CAE5ih79DuU5Wrk4p+wpQX3j6bgyA2_ZgXYJX-tGvZpYhWazWqA@mail.gmail.com","subject":"Re: could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-02-08T10:02:22Z","receivedAt":"2019-02-08T10:02:29Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Feb 06, 2019 at 10:25:25AM +0000, Luke Diamand wrote:\n> I've recently started seeing a lot of this message when doing a rebase:\n> \n> warning: could not freshen shared index\n> '/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.0000000000000000000000000000000000000000'\n\nThere are only two places in the code that could print this. The one\nin read_index_from() can't happen unless is_null_oid() is broken (very\nvery unlikely).\n\nThe other one is in write_locked_index() which could happen in theory\nbut I don't understand how it got there. If you could build git, could\nyou try this patch and see if it helps?\n\n-- 8< --\ndiff --git a/read-cache.c b/read-cache.c\nindex f68b367613..5ad71478dc 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -3165,6 +3165,7 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \t\tfill_fsmonitor_bitmap(istate);\n \n \tif (!si || alternate_index_output ||\n+\t (si && is_null_oid(&si->base_oid)) ||\n \t (istate->cache_changed & ~EXTMASK)) {\n \t\tif (si)\n \t\t\toidclr(&si->base_oid);\n-- 8< --\n\n\n> (There's a repo called dev_full, and I've got a worktree where I'm\n> working on my 3rd attempt to make it work with gcc8).\n> \n> That file doesn't actually exist but there are a bunch of\n> sharedindex.XXX files in there with more convincing looking names.\n> \n> 2.20.1.611.gfbb209baf1\n--\nDuy\n"},{"id":"368838","messageId":"CAE5ih7_b_LXfPp5eMcrr4iSuyzaiKxvQqN73Osw+ck9CPrfdqw@mail.gmail.com","threadId":"50409","inReplyTo":"20190208100221.GA22283@ash","subject":"Re: could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2019-02-08T16:38:49Z","receivedAt":"2019-02-08T16:39:09Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On Fri, 8 Feb 2019 at 10:02, Duy Nguyen wrote:\n>\n> On Wed, Feb 06, 2019 at 10:25:25AM +0000, Luke Diamand wrote:\n> > I've recently started seeing a lot of this message when doing a rebase:\n> >\n> > warning: could not freshen shared index\n> > '/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.0000000000000000000000000000000000000000'\n>\n> There are only two places in the code that could print this. The one\n> in read_index_from() can't happen unless is_null_oid() is broken (very\n> very unlikely).\n>\n> The other one is in write_locked_index() which could happen in theory\n> but I don't understand how it got there. If you could build git, could\n> you try this patch and see if it helps?\n\nThey've gone away!\n\n\n>\n> -- 8< --\n> diff --git a/read-cache.c b/read-cache.c\n> index f68b367613..5ad71478dc 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -3165,6 +3165,7 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n> fill_fsmonitor_bitmap(istate);\n>\n> if (!si || alternate_index_output ||\n> + (si && is_null_oid(&si->base_oid)) ||\n> (istate->cache_changed & ~EXTMASK)) {\n> if (si)\n> oidclr(&si->base_oid);\n> -- 8< --\n>\n\nLuke\n"},{"id":"368842","messageId":"xmqqzhr6kxcj.fsf@gitster-ct.c.googlers.com","threadId":"50409","inReplyTo":"20190208100221.GA22283@ash","subject":"Re: could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-08T17:23:56Z","receivedAt":"2019-02-08T17:24:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Wed, Feb 06, 2019 at 10:25:25AM +0000, Luke Diamand wrote:\n>> I've recently started seeing a lot of this message when doing a rebase:\n>> \n>> warning: could not freshen shared index\n>> '/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.0000000000000000000000000000000000000000'\n>\n> There are only two places in the code that could print this. The one\n> in read_index_from() can't happen unless is_null_oid() is broken (very\n> very unlikely).\n>\n> The other one is in write_locked_index() which could happen in theory\n> but I don't understand how it got there. If you could build git, could\n> you try this patch and see if it helps?\n\n... meaning, if it hides the symptom, we'd know the codepath that\ncauses a NULL si->base_oid to appear here is the culprit? Or do you\nmean that it is expected si->base_oid sometimes is NULL and we should\nhave been pretending as if si were NULL (i.e. split_index is not being\nused)?\n\nI take it as the latter (i.e. \"helps\" to narrow down the bug hunting\nfield, not \"helps\" by fixing the bug).\n\nThanks.\n\n>\n> -- 8< --\n> diff --git a/read-cache.c b/read-cache.c\n> index f68b367613..5ad71478dc 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -3165,6 +3165,7 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n> \t\tfill_fsmonitor_bitmap(istate);\n> \n> \tif (!si || alternate_index_output ||\n> +\t (si && is_null_oid(&si->base_oid)) ||\n> \t (istate->cache_changed & ~EXTMASK)) {\n> \t\tif (si)\n> \t\t\toidclr(&si->base_oid);\n> -- 8< --\n>\n>\n>> (There's a repo called dev_full, and I've got a worktree where I'm\n>> working on my 3rd attempt to make it work with gcc8).\n>> \n>> That file doesn't actually exist but there are a bunch of\n>> sharedindex.XXX files in there with more convincing looking names.\n>> \n>> 2.20.1.611.gfbb209baf1\n> --\n> Duy\n"},{"id":"368907","messageId":"CACsJy8AzQLvWNq4hGGva=W4CKJAKHXiopF-cVw--wdJE2npdGQ@mail.gmail.com","threadId":"50409","inReplyTo":"xmqqzhr6kxcj.fsf@gitster-ct.c.googlers.com","subject":"Re: could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-02-09T04:56:47Z","receivedAt":"2019-02-09T04:57:16Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Feb 9, 2019 at 12:24 AM Junio C Hamano wrote:\n>\n> Duy Nguyen writes:\n>\n> > On Wed, Feb 06, 2019 at 10:25:25AM +0000, Luke Diamand wrote:\n> >> I've recently started seeing a lot of this message when doing a rebase:\n> >>\n> >> warning: could not freshen shared index\n> >> '/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.0000000000000000000000000000000000000000'\n> >\n> > There are only two places in the code that could print this. The one\n> > in read_index_from() can't happen unless is_null_oid() is broken (very\n> > very unlikely).\n> >\n> > The other one is in write_locked_index() which could happen in theory\n> > but I don't understand how it got there. If you could build git, could\n> > you try this patch and see if it helps?\n>\n> ... meaning, if it hides the symptom, we'd know the codepath that\n> causes a NULL si->base_oid to appear here is the culprit? Or do you\n> mean that it is expected si->base_oid sometimes is NULL and we should\n> have been pretending as if si were NULL (i.e. split_index is not being\n> used)?\n\nI didn't go that far when I suggested my patch. Looking more at the\ncode, I think we may have written the \"link\" extension with null oid.\nWhich could trigger this case and is definitely wrong.\n\n> I take it as the latter (i.e. \"helps\" to narrow down the bug hunting\n> field, not \"helps\" by fixing the bug).\n\nYeah definitely not the fix (how can I write \"I don't know why\" in the\ncommit message for the fix). At least now I know I'm on the right\ntrack.\n\nSzeder recently fixed some racy bug in split-index.c, which seems fit\nin this rebase scenario since we'll be updating the index very often\nin a short period of time. Perhaps that uncovers some case that we\ndon't handle well. I haven't been able to connect the dots though.\n-- \nDuy\n"},{"id":"368908","messageId":"CACsJy8CeRFyhkdohwU_KtXtgmkOzNsQPb4oKFcn+R210MpS1KA@mail.gmail.com","threadId":"50409","inReplyTo":"CAE5ih7_b_LXfPp5eMcrr4iSuyzaiKxvQqN73Osw+ck9CPrfdqw@mail.gmail.com","subject":"Re: could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-02-09T05:00:59Z","receivedAt":"2019-02-09T05:01:28Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Feb 8, 2019 at 11:39 PM Luke Diamand wrote:\n>\n> On Fri, 8 Feb 2019 at 10:02, Duy Nguyen wrote:\n> >\n> > On Wed, Feb 06, 2019 at 10:25:25AM +0000, Luke Diamand wrote:\n> > > I've recently started seeing a lot of this message when doing a rebase:\n> > >\n> > > warning: could not freshen shared index\n> > > '/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.0000000000000000000000000000000000000000'\n> >\n> > There are only two places in the code that could print this. The one\n> > in read_index_from() can't happen unless is_null_oid() is broken (very\n> > very unlikely).\n> >\n> > The other one is in write_locked_index() which could happen in theory\n> > but I don't understand how it got there. If you could build git, could\n> > you try this patch and see if it helps?\n>\n> They've gone away!\n\nGreat! Since you seem able to reproduce this (and can build git!)\ncould you try bisect to pin point the commit that causes this problem?\nThat would help a lot. I think you could start maybe from v2.19.0\n-- \nDuy\n"},{"id":"368916","messageId":"CAE5ih79JYrcV9cxMBU88Hq=HHQOOyzpkq+kT2zAgDzs=ao+PMg@mail.gmail.com","threadId":"50409","inReplyTo":"CACsJy8CeRFyhkdohwU_KtXtgmkOzNsQPb4oKFcn+R210MpS1KA@mail.gmail.com","subject":"Re: could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2019-02-09T09:57:51Z","receivedAt":"2019-02-09T09:58:06Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On Sat, 9 Feb 2019 at 05:01, Duy Nguyen wrote:\n>\n> On Fri, Feb 8, 2019 at 11:39 PM Luke Diamand wrote:\n> >\n> > On Fri, 8 Feb 2019 at 10:02, Duy Nguyen wrote:\n> > >\n> > > On Wed, Feb 06, 2019 at 10:25:25AM +0000, Luke Diamand wrote:\n> > > > I've recently started seeing a lot of this message when doing a rebase:\n> > > >\n> > > > warning: could not freshen shared index\n> > > > '/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.0000000000000000000000000000000000000000'\n> > >\n> > > There are only two places in the code that could print this. The one\n> > > in read_index_from() can't happen unless is_null_oid() is broken (very\n> > > very unlikely).\n> > >\n> > > The other one is in write_locked_index() which could happen in theory\n> > > but I don't understand how it got there. If you could build git, could\n> > > you try this patch and see if it helps?\n> >\n> > They've gone away!\n>\n> Great! Since you seem able to reproduce this (and can build git!)\n> could you try bisect to pin point the commit that causes this problem?\n> That would help a lot. I think you could start maybe from v2.19.0\n\nThe first bad commit was d658196f3c4b2478ebdff638e2c3e4fb2f9cbba2.\n\n$ git show d658196f3c4b2478ebdff638e2c3e4fb2f9cbba2\ncommit d658196f3c4b2478ebdff638e2c3e4fb2f9cbba2 (refs/bisect/bad)\nMerge: 6b0f1d9c47 7db118303a\nAuthor: Junio C Hamano \nDate: Wed May 23 14:38:22 2018 +0900\n\n Merge branch 'en/unpack-trees-split-index-fix'\n\n The split-index feature had a long-standing and dormant bug in\n certain use of the in-core merge machinery, which has been fixed.\n\n * en/unpack-trees-split-index-fix:\n unpack_trees: fix breakage when o->src_index != o->dst_index\n\nThe test I'm doing is just:\n\n1. git checkout some_tag\n2. git rebase -i HEAD~5\n3. Swap the top and bottom commit\n4. repeat\n\nI just chose \"5\" as my first wild guess, other numbers are also available.\n\nWith \"5\" I get 3 lots of:\n warning: could not freshen shared index\n'.git/sharedindex.0000000000000000000000000000000000000000'\n\nAs far as I can tell the actual rebasing is fine.\n"},{"id":"368917","messageId":"CACsJy8A7A+Fu=5PWGuQsxtdJLBP_2Ueb-1MCKGHwKHGoQ7J2GA@mail.gmail.com","threadId":"50409","inReplyTo":"CAE5ih79JYrcV9cxMBU88Hq=HHQOOyzpkq+kT2zAgDzs=ao+PMg@mail.gmail.com","subject":"Re: could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-02-09T10:36:50Z","receivedAt":"2019-02-09T10:37:19Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Feb 9, 2019 at 4:58 PM Luke Diamand wrote:\n>\n> On Sat, 9 Feb 2019 at 05:01, Duy Nguyen wrote:\n> >\n> > On Fri, Feb 8, 2019 at 11:39 PM Luke Diamand wrote:\n> > >\n> > > On Fri, 8 Feb 2019 at 10:02, Duy Nguyen wrote:\n> > > >\n> > > > On Wed, Feb 06, 2019 at 10:25:25AM +0000, Luke Diamand wrote:\n> > > > > I've recently started seeing a lot of this message when doing a rebase:\n> > > > >\n> > > > > warning: could not freshen shared index\n> > > > > '/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.0000000000000000000000000000000000000000'\n> > > >\n> > > > There are only two places in the code that could print this. The one\n> > > > in read_index_from() can't happen unless is_null_oid() is broken (very\n> > > > very unlikely).\n> > > >\n> > > > The other one is in write_locked_index() which could happen in theory\n> > > > but I don't understand how it got there. If you could build git, could\n> > > > you try this patch and see if it helps?\n> > >\n> > > They've gone away!\n> >\n> > Great! Since you seem able to reproduce this (and can build git!)\n> > could you try bisect to pin point the commit that causes this problem?\n> > That would help a lot. I think you could start maybe from v2.19.0\n>\n> The first bad commit was d658196f3c4b2478ebdff638e2c3e4fb2f9cbba2.\n\nThanks! I could reproduce it and I think I know what the problem is\nnow. Patches coming soon.\n\n>\n> $ git show d658196f3c4b2478ebdff638e2c3e4fb2f9cbba2\n> commit d658196f3c4b2478ebdff638e2c3e4fb2f9cbba2 (refs/bisect/bad)\n> Merge: 6b0f1d9c47 7db118303a\n> Author: Junio C Hamano \n> Date: Wed May 23 14:38:22 2018 +0900\n>\n> Merge branch 'en/unpack-trees-split-index-fix'\n>\n> The split-index feature had a long-standing and dormant bug in\n> certain use of the in-core merge machinery, which has been fixed.\n>\n> * en/unpack-trees-split-index-fix:\n> unpack_trees: fix breakage when o->src_index != o->dst_index\n>\n> The test I'm doing is just:\n>\n> 1. git checkout some_tag\n> 2. git rebase -i HEAD~5\n> 3. Swap the top and bottom commit\n> 4. repeat\n>\n> I just chose \"5\" as my first wild guess, other numbers are also available.\n>\n> With \"5\" I get 3 lots of:\n> warning: could not freshen shared index\n> '.git/sharedindex.0000000000000000000000000000000000000000'\n>\n> As far as I can tell the actual rebasing is fine.\n\n\n\n-- \nDuy\n"},{"id":"368918","messageId":"20190209112328.26317-1-pclouds@gmail.com","threadId":"50409","inReplyTo":"CAE5ih79JYrcV9cxMBU88Hq=HHQOOyzpkq+kT2zAgDzs=ao+PMg@mail.gmail.com","subject":"[PATCH] unpack-trees.c: fix writing \"link\" index ext with null base oid","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-02-09T11:23:28Z","receivedAt":"2019-02-09T11:23:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Since commit 7db118303a (unpack_trees: fix breakage when o->src_index !=\no->dst_index - 2018-04-23) and changes in merge code to use separate\nindex_state for source and destination, when doing a merge with split\nindex activated, we may run into this line in unpack_trees():\n\n o->result.split_index = init_split_index(&o->result);\n\nThis is by itself not wrong. But this split index information is not\nfully populated (and it's only so when move_cache_to_base_index() is\ncalled, aka force splitting the index, or loading index_state from a\nfile). Both \"base_oid\" and \"base\" in this case remain null.\n\nSo when writing the main index down, we link to this index with null\noid (default value after init_split_index()), which also means \"no split\nindex\" internally. This triggers an incorrect base index refresh:\n\n warning: could not freshen shared index '.../sharedindex.0{40}'\n\nThis patch makes sure we will not refresh null base_oid (because the\nfile is never there). It also makes sure not to write \"link\" extension\nwith null base_oid in the first place (no point having it at\nall). Read code already has protection against null base_oid.\n\nThere is also another side fix in remove_split_index() that causes a\ncrash when doing \"git update-index --no-split-index\" when base_oid in\nthe index file is null. In this case we will not load\nistate->split_index->base but we dereference it anyway and are rewarded\nwith a segfault. This should not happen anymore, but it's still wrong to\ndereference a potential NULL pointer, especially when we do check for\nNULL pointer in the next code.\n\nReported-by: Luke Diamand \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n I considered adding a test, but since the problem is a warning, not\n sure how to catch that. And a test would not be able to verify all\n changes in this patch anyway.\n\n read-cache.c | 5 +++--\n split-index.c | 34 ++++++++++++++++++----------------\n 2 files changed, 21 insertions(+), 18 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 0e0c93edc9..d6fb09984f 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2894,7 +2894,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\t\treturn -1;\n \t}\n \n-\tif (!strip_extensions && istate->split_index) {\n+\tif (!strip_extensions && istate->split_index &&\n+\t !is_null_oid(&istate->split_index->base_oid)) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \n \t\terr = write_link_extension(&sb, istate) < 0 ||\n@@ -3189,7 +3190,7 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \tret = write_split_index(istate, lock, flags);\n \n \t/* Freshen the shared index only if the split-index was written */\n-\tif (!ret && !new_shared_index) {\n+\tif (!ret && !new_shared_index && !is_null_oid(&si->base_oid)) {\n \t\tconst char *shared_index = git_path(\"sharedindex.%s\",\n \t\t\t\t\t\t oid_to_hex(&si->base_oid));\n \t\tfreshen_shared_index(shared_index, 1);\ndiff --git a/split-index.c b/split-index.c\nindex 5820412dc5..a9d13611a4 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -440,24 +440,26 @@ void add_split_index(struct index_state *istate)\n void remove_split_index(struct index_state *istate)\n {\n \tif (istate->split_index) {\n-\t\t/*\n-\t\t * When removing the split index, we need to move\n-\t\t * ownership of the mem_pool associated with the\n-\t\t * base index to the main index. There may be cache entries\n-\t\t * allocated from the base's memory pool that are shared with\n-\t\t * the_index.cache[].\n-\t\t */\n-\t\tmem_pool_combine(istate->ce_mem_pool, istate->split_index->base->ce_mem_pool);\n+\t\tif (istate->split_index->base) {\n+\t\t\t/*\n+\t\t\t * When removing the split index, we need to move\n+\t\t\t * ownership of the mem_pool associated with the\n+\t\t\t * base index to the main index. There may be cache entries\n+\t\t\t * allocated from the base's memory pool that are shared with\n+\t\t\t * the_index.cache[].\n+\t\t\t */\n+\t\t\tmem_pool_combine(istate->ce_mem_pool,\n+\t\t\t\t\t istate->split_index->base->ce_mem_pool);\n \n-\t\t/*\n-\t\t * The split index no longer owns the mem_pool backing\n-\t\t * its cache array. As we are discarding this index,\n-\t\t * mark the index as having no cache entries, so it\n-\t\t * will not attempt to clean up the cache entries or\n-\t\t * validate them.\n-\t\t */\n-\t\tif (istate->split_index->base)\n+\t\t\t/*\n+\t\t\t * The split index no longer owns the mem_pool backing\n+\t\t\t * its cache array. As we are discarding this index,\n+\t\t\t * mark the index as having no cache entries, so it\n+\t\t\t * will not attempt to clean up the cache entries or\n+\t\t\t * validate them.\n+\t\t\t */\n \t\t\tistate->split_index->base->cache_nr = 0;\n+\t\t}\n \n \t\t/*\n \t\t * We can discard the split index because its\n-- \n2.20.1.682.gd5861c6d90\n\n"},{"id":"368921","messageId":"CAE5ih7-0Qq6DOrE0Y7jQ++Jer0P5SGM38KA0n1_busAi08ux=Q@mail.gmail.com","threadId":"50409","inReplyTo":"20190209112328.26317-1-pclouds@gmail.com","subject":"Re: [PATCH] unpack-trees.c: fix writing \"link\" index ext with null base oid","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2019-02-09T14:14:25Z","receivedAt":"2019-02-09T14:14:40Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On Sat, 9 Feb 2019 at 11:23, Nguyễn Thái Ngọc Duy wrote:\n>\n> Since commit 7db118303a (unpack_trees: fix breakage when o->src_index !=\n> o->dst_index - 2018-04-23) and changes in merge code to use separate\n> index_state for source and destination, when doing a merge with split\n> index activated, we may run into this line in unpack_trees():\n>\n> o->result.split_index = init_split_index(&o->result);\n>\n> This is by itself not wrong. But this split index information is not\n> fully populated (and it's only so when move_cache_to_base_index() is\n> called, aka force splitting the index, or loading index_state from a\n> file). Both \"base_oid\" and \"base\" in this case remain null.\n>\n> So when writing the main index down, we link to this index with null\n> oid (default value after init_split_index()), which also means \"no split\n> index\" internally. This triggers an incorrect base index refresh:\n>\n> warning: could not freshen shared index '.../sharedindex.0{40}'\n>\n> This patch makes sure we will not refresh null base_oid (because the\n> file is never there). It also makes sure not to write \"link\" extension\n> with null base_oid in the first place (no point having it at\n> all). Read code already has protection against null base_oid.\n>\n> There is also another side fix in remove_split_index() that causes a\n> crash when doing \"git update-index --no-split-index\" when base_oid in\n> the index file is null. In this case we will not load\n> istate->split_index->base but we dereference it anyway and are rewarded\n> with a segfault. This should not happen anymore, but it's still wrong to\n> dereference a potential NULL pointer, especially when we do check for\n> NULL pointer in the next code.\n>\n> Reported-by: Luke Diamand \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> I considered adding a test, but since the problem is a warning, not\n> sure how to catch that. And a test would not be able to verify all\n> changes in this patch anyway.\n>\n> read-cache.c | 5 +++--\n> split-index.c | 34 ++++++++++++++++++----------------\n> 2 files changed, 21 insertions(+), 18 deletions(-)\n>\n> diff --git a/read-cache.c b/read-cache.c\n> index 0e0c93edc9..d6fb09984f 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -2894,7 +2894,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n> return -1;\n> }\n>\n> - if (!strip_extensions && istate->split_index) {\n> + if (!strip_extensions && istate->split_index &&\n> + !is_null_oid(&istate->split_index->base_oid)) {\n> struct strbuf sb = STRBUF_INIT;\n>\n> err = write_link_extension(&sb, istate) < 0 ||\n> @@ -3189,7 +3190,7 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n> ret = write_split_index(istate, lock, flags);\n>\n> /* Freshen the shared index only if the split-index was written */\n> - if (!ret && !new_shared_index) {\n> + if (!ret && !new_shared_index && !is_null_oid(&si->base_oid)) {\n> const char *shared_index = git_path(\"sharedindex.%s\",\n> oid_to_hex(&si->base_oid));\n> freshen_shared_index(shared_index, 1);\n> diff --git a/split-index.c b/split-index.c\n> index 5820412dc5..a9d13611a4 100644\n> --- a/split-index.c\n> +++ b/split-index.c\n> @@ -440,24 +440,26 @@ void add_split_index(struct index_state *istate)\n> void remove_split_index(struct index_state *istate)\n> {\n> if (istate->split_index) {\n> - /*\n> - * When removing the split index, we need to move\n> - * ownership of the mem_pool associated with the\n> - * base index to the main index. There may be cache entries\n> - * allocated from the base's memory pool that are shared with\n> - * the_index.cache[].\n> - */\n> - mem_pool_combine(istate->ce_mem_pool, istate->split_index->base->ce_mem_pool);\n> + if (istate->split_index->base) {\n> + /*\n> + * When removing the split index, we need to move\n> + * ownership of the mem_pool associated with the\n> + * base index to the main index. There may be cache entries\n> + * allocated from the base's memory pool that are shared with\n> + * the_index.cache[].\n> + */\n> + mem_pool_combine(istate->ce_mem_pool,\n> + istate->split_index->base->ce_mem_pool);\n>\n> - /*\n> - * The split index no longer owns the mem_pool backing\n> - * its cache array. As we are discarding this index,\n> - * mark the index as having no cache entries, so it\n> - * will not attempt to clean up the cache entries or\n> - * validate them.\n> - */\n> - if (istate->split_index->base)\n> + /*\n> + * The split index no longer owns the mem_pool backing\n> + * its cache array. As we are discarding this index,\n> + * mark the index as having no cache entries, so it\n> + * will not attempt to clean up the cache entries or\n> + * validate them.\n> + */\n> istate->split_index->base->cache_nr = 0;\n> + }\n>\n> /*\n> * We can discard the split index because its\n> --\n> 2.20.1.682.gd5861c6d90\n\nAck, I no longer see those messages.\n\n\n>\n"},{"id":"369128","messageId":"CABPp-BF+q0QY1YAKOk4iSrCRuEc7xsG7iqCzeDD9v8+jmvQvzg@mail.gmail.com","threadId":"50409","inReplyTo":"20190209112328.26317-1-pclouds@gmail.com","subject":"Re: [PATCH] unpack-trees.c: fix writing \"link\" index ext with null base oid","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-12T05:43:35Z","receivedAt":"2019-02-12T05:46:36Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sat, Feb 9, 2019 at 3:23 AM Nguyễn Thái Ngọc Duy wrote:\n>\n> Since commit 7db118303a (unpack_trees: fix breakage when o->src_index !=\n> o->dst_index - 2018-04-23) and changes in merge code to use separate\n> index_state for source and destination, when doing a merge with split\n> index activated, we may run into this line in unpack_trees():\n>\n> o->result.split_index = init_split_index(&o->result);\n>\n> This is by itself not wrong. But this split index information is not\n> fully populated (and it's only so when move_cache_to_base_index() is\n> called, aka force splitting the index, or loading index_state from a\n> file). Both \"base_oid\" and \"base\" in this case remain null.\n>\n> So when writing the main index down, we link to this index with null\n> oid (default value after init_split_index()), which also means \"no split\n> index\" internally. This triggers an incorrect base index refresh:\n>\n> warning: could not freshen shared index '.../sharedindex.0{40}'\n>\n> This patch makes sure we will not refresh null base_oid (because the\n> file is never there). It also makes sure not to write \"link\" extension\n> with null base_oid in the first place (no point having it at\n> all). Read code already has protection against null base_oid.\n>\n> There is also another side fix in remove_split_index() that causes a\n> crash when doing \"git update-index --no-split-index\" when base_oid in\n> the index file is null. In this case we will not load\n> istate->split_index->base but we dereference it anyway and are rewarded\n> with a segfault. This should not happen anymore, but it's still wrong to\n> dereference a potential NULL pointer, especially when we do check for\n> NULL pointer in the next code.\n>\n> Reported-by: Luke Diamand \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n\nThanks for digging this down and fixing it. When I saw this split\nindex bug bisect to me that my heart sank a little; there's so much of\nthat code I don't understand. Nice to see you've already come along\nand fixed it all up. :-)\n\n> ---\n> I considered adding a test, but since the problem is a warning, not\n> sure how to catch that. And a test would not be able to verify all\n> changes in this patch anyway.\n>\n> read-cache.c | 5 +++--\n> split-index.c | 34 ++++++++++++++++++----------------\n> 2 files changed, 21 insertions(+), 18 deletions(-)\n>\n> diff --git a/read-cache.c b/read-cache.c\n> index 0e0c93edc9..d6fb09984f 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -2894,7 +2894,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n> return -1;\n> }\n>\n> - if (!strip_extensions && istate->split_index) {\n> + if (!strip_extensions && istate->split_index &&\n> + !is_null_oid(&istate->split_index->base_oid)) {\n> struct strbuf sb = STRBUF_INIT;\n>\n> err = write_link_extension(&sb, istate) < 0 ||\n> @@ -3189,7 +3190,7 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n> ret = write_split_index(istate, lock, flags);\n>\n> /* Freshen the shared index only if the split-index was written */\n> - if (!ret && !new_shared_index) {\n> + if (!ret && !new_shared_index && !is_null_oid(&si->base_oid)) {\n> const char *shared_index = git_path(\"sharedindex.%s\",\n> oid_to_hex(&si->base_oid));\n> freshen_shared_index(shared_index, 1);\n> diff --git a/split-index.c b/split-index.c\n> index 5820412dc5..a9d13611a4 100644\n> --- a/split-index.c\n> +++ b/split-index.c\n> @@ -440,24 +440,26 @@ void add_split_index(struct index_state *istate)\n> void remove_split_index(struct index_state *istate)\n> {\n> if (istate->split_index) {\n> - /*\n> - * When removing the split index, we need to move\n> - * ownership of the mem_pool associated with the\n> - * base index to the main index. There may be cache entries\n> - * allocated from the base's memory pool that are shared with\n> - * the_index.cache[].\n> - */\n> - mem_pool_combine(istate->ce_mem_pool, istate->split_index->base->ce_mem_pool);\n> + if (istate->split_index->base) {\n> + /*\n> + * When removing the split index, we need to move\n> + * ownership of the mem_pool associated with the\n> + * base index to the main index. There may be cache entries\n> + * allocated from the base's memory pool that are shared with\n> + * the_index.cache[].\n> + */\n> + mem_pool_combine(istate->ce_mem_pool,\n> + istate->split_index->base->ce_mem_pool);\n>\n> - /*\n> - * The split index no longer owns the mem_pool backing\n> - * its cache array. As we are discarding this index,\n> - * mark the index as having no cache entries, so it\n> - * will not attempt to clean up the cache entries or\n> - * validate them.\n> - */\n> - if (istate->split_index->base)\n> + /*\n> + * The split index no longer owns the mem_pool backing\n> + * its cache array. As we are discarding this index,\n> + * mark the index as having no cache entries, so it\n> + * will not attempt to clean up the cache entries or\n> + * validate them.\n> + */\n> istate->split_index->base->cache_nr = 0;\n> + }\n>\n> /*\n> * We can discard the split index because its\n> --\n> 2.20.1.682.gd5861c6d90\n>\n"},{"id":"369135","messageId":"8736otl56m.fsf@evledraar.gmail.com","threadId":"50409","inReplyTo":"20190209112328.26317-1-pclouds@gmail.com","subject":"Re: [PATCH] unpack-trees.c: fix writing \"link\" index ext with null base oid","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-02-12T09:36:01Z","receivedAt":"2019-02-12T09:36:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Feb 09 2019, Nguyễn Thái Ngọc Duy wrote:\n\nI ran into this bug in the past but wasn't able to reproduce it long\nenough to track it down. Thanks for the fix.\n\nIt would be nice to have a test for it if it's easily narrowed down,\ntricky edge cases we fix are worth testing for...\n\n> I considered adding a test, but since the problem is a warning, not\n> sure how to catch that. And a test would not be able to verify all\n> changes in this patch anyway.\n\nYou'd write the test like this:\n\n git [...] 2>stderr &&\n test_i18ngrep \"could not.*000000000\" stderr\n\nOr something like that. We have a lot of tests like this already, some\ncan be found with:\n\n git grep -B1 test_i18ngrep.*stderr\n\nSo something being a warning shouldn't stop us from testing for it.\n"},{"id":"369165","messageId":"xmqq1s4deyuw.fsf@gitster-ct.c.googlers.com","threadId":"50409","inReplyTo":"CABPp-BF+q0QY1YAKOk4iSrCRuEc7xsG7iqCzeDD9v8+jmvQvzg@mail.gmail.com","subject":"Re: [PATCH] unpack-trees.c: fix writing \"link\" index ext with null base oid","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-12T16:49:11Z","receivedAt":"2019-02-12T16:49:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n>> There is also another side fix in remove_split_index() that causes a\n>> crash when doing \"git update-index --no-split-index\" when base_oid in\n>> the index file is null. In this case we will not load\n>> istate->split_index->base but we dereference it anyway and are rewarded\n>> with a segfault. This should not happen anymore, but it's still wrong to\n>> dereference a potential NULL pointer, especially when we do check for\n>> NULL pointer in the next code.\n>>\n>> Reported-by: Luke Diamand \n>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>\n> Thanks for digging this down and fixing it. When I saw this split\n> index bug bisect to me that my heart sank a little; there's so much of\n> that code I don't understand. Nice to see you've already come along\n> and fixed it all up. :-)\n\nThanks, all.\n"},{"id":"369214","messageId":"20190213095129.31272-1-pclouds@gmail.com","threadId":"50409","inReplyTo":"20190209112328.26317-1-pclouds@gmail.com","subject":"[PATCH v2] read-cache.c: fix writing \"link\" index ext with null base oid","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-02-13T09:51:29Z","receivedAt":"2019-02-13T09:51:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Since commit 7db118303a (unpack_trees: fix breakage when o->src_index !=\no->dst_index - 2018-04-23) and changes in merge code to use separate\nindex_state for source and destination, when doing a merge with split\nindex activated, we may run into this line in unpack_trees():\n\n o->result.split_index = init_split_index(&o->result);\n\nThis is by itself not wrong. But this split index information is not\nfully populated (and it's only so when move_cache_to_base_index() is\ncalled, aka force splitting the index, or loading index_state from a\nfile). Both \"base_oid\" and \"base\" in this case remain null.\n\nSo when writing the main index down, we link to this index with null\noid (default value after init_split_index()), which also means \"no split\nindex\" internally. This triggers an incorrect base index refresh:\n\n warning: could not freshen shared index '.../sharedindex.0{40}'\n\nThis patch makes sure we will not refresh null base_oid (because the\nfile is never there). It also makes sure not to write \"link\" extension\nwith null base_oid in the first place (no point having it at\nall). Read code already has protection against null base_oid.\n\nThere is also another side fix in remove_split_index() that causes a\ncrash when doing \"git update-index --no-split-index\" when base_oid in\nthe index file is null. In this case we will not load\nistate->split_index->base but we dereference it anyway and are rewarded\nwith a segfault. This should not happen anymore, but it's still wrong to\ndereference a potential NULL pointer, especially when we do check for\nNULL pointer in the next code.\n\nReported-by: Luke Diamand \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n v2 added a new test\n\n read-cache.c | 5 +++--\n split-index.c | 34 ++++++++++++++++++----------------\n t/t1700-split-index.sh | 18 ++++++++++++++++++\n 3 files changed, 39 insertions(+), 18 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 8f644f68b4..d140b44f8f 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2520,7 +2520,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\treturn err;\n \n \t/* Write extension data here */\n-\tif (!strip_extensions && istate->split_index) {\n+\tif (!strip_extensions && istate->split_index &&\n+\t !is_null_oid(&istate->split_index->base_oid)) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \n \t\terr = write_link_extension(&sb, istate) < 0 ||\n@@ -2794,7 +2795,7 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \tret = write_split_index(istate, lock, flags);\n \n \t/* Freshen the shared index only if the split-index was written */\n-\tif (!ret && !new_shared_index) {\n+\tif (!ret && !new_shared_index && !is_null_oid(&si->base_oid)) {\n \t\tconst char *shared_index = git_path(\"sharedindex.%s\",\n \t\t\t\t\t\t oid_to_hex(&si->base_oid));\n \t\tfreshen_shared_index(shared_index, 1);\ndiff --git a/split-index.c b/split-index.c\nindex 5820412dc5..a9d13611a4 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -440,24 +440,26 @@ void add_split_index(struct index_state *istate)\n void remove_split_index(struct index_state *istate)\n {\n \tif (istate->split_index) {\n-\t\t/*\n-\t\t * When removing the split index, we need to move\n-\t\t * ownership of the mem_pool associated with the\n-\t\t * base index to the main index. There may be cache entries\n-\t\t * allocated from the base's memory pool that are shared with\n-\t\t * the_index.cache[].\n-\t\t */\n-\t\tmem_pool_combine(istate->ce_mem_pool, istate->split_index->base->ce_mem_pool);\n+\t\tif (istate->split_index->base) {\n+\t\t\t/*\n+\t\t\t * When removing the split index, we need to move\n+\t\t\t * ownership of the mem_pool associated with the\n+\t\t\t * base index to the main index. There may be cache entries\n+\t\t\t * allocated from the base's memory pool that are shared with\n+\t\t\t * the_index.cache[].\n+\t\t\t */\n+\t\t\tmem_pool_combine(istate->ce_mem_pool,\n+\t\t\t\t\t istate->split_index->base->ce_mem_pool);\n \n-\t\t/*\n-\t\t * The split index no longer owns the mem_pool backing\n-\t\t * its cache array. As we are discarding this index,\n-\t\t * mark the index as having no cache entries, so it\n-\t\t * will not attempt to clean up the cache entries or\n-\t\t * validate them.\n-\t\t */\n-\t\tif (istate->split_index->base)\n+\t\t\t/*\n+\t\t\t * The split index no longer owns the mem_pool backing\n+\t\t\t * its cache array. As we are discarding this index,\n+\t\t\t * mark the index as having no cache entries, so it\n+\t\t\t * will not attempt to clean up the cache entries or\n+\t\t\t * validate them.\n+\t\t\t */\n \t\t\tistate->split_index->base->cache_nr = 0;\n+\t\t}\n \n \t\t/*\n \t\t * We can discard the split index because its\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex f053bf83eb..ea5181aff9 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -447,4 +447,22 @@ test_expect_success 'writing split index with null sha1 does not write cache tre\n \ttest_line_count = 0 cache-tree.out\n '\n \n+test_expect_success 'do not refresh null base index' '\n+\ttest_create_repo merge &&\n+\t(\n+\t\tcd merge &&\n+\t\ttest_commit initial &&\n+\t\tgit checkout -b side-branch &&\n+\t\ttest_commit extra &&\n+\t\tgit checkout master &&\n+\t\tgit update-index --split-index &&\n+\t\ttest_commit more &&\n+\t\t# must not write a new shareindex, or we wont catch the problem\n+\t\tgit -c splitIndex.maxPercentChange=100 merge --no-edit side-branch 2>err &&\n+\t\t# i.e. do not expect warnings like\n+\t\t# could not freshen shared index .../shareindex.00000...\n+\t\ttest_must_be_empty err\n+\t)\n+'\n+\n test_done\n-- \n2.21.0.rc0.328.g0e39304f8d\n\n"},{"id":"369229","messageId":"87imxnkhqb.fsf@evledraar.gmail.com","threadId":"50409","inReplyTo":"20190213095129.31272-1-pclouds@gmail.com","subject":"Re: [PATCH v2] read-cache.c: fix writing \"link\" index ext with null base oid","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-02-13T12:14:52Z","receivedAt":"2019-02-13T12:15:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Feb 13 2019, Nguyễn Thái Ngọc Duy wrote:\n\n> Since commit 7db118303a (unpack_trees: fix breakage when o->src_index !=\n> o->dst_index - 2018-04-23) and changes in merge code to use separate\n> index_state for source and destination, when doing a merge with split\n> index activated, we may run into this line in unpack_trees():\n>\n> o->result.split_index = init_split_index(&o->result);\n>\n> This is by itself not wrong. But this split index information is not\n> fully populated (and it's only so when move_cache_to_base_index() is\n> called, aka force splitting the index, or loading index_state from a\n> file). Both \"base_oid\" and \"base\" in this case remain null.\n>\n> So when writing the main index down, we link to this index with null\n> oid (default value after init_split_index()), which also means \"no split\n> index\" internally. This triggers an incorrect base index refresh:\n>\n> warning: could not freshen shared index '.../sharedindex.0{40}'\n>\n> This patch makes sure we will not refresh null base_oid (because the\n> file is never there). It also makes sure not to write \"link\" extension\n> with null base_oid in the first place (no point having it at\n> all). Read code already has protection against null base_oid.\n>\n> There is also another side fix in remove_split_index() that causes a\n> crash when doing \"git update-index --no-split-index\" when base_oid in\n> the index file is null. In this case we will not load\n> istate->split_index->base but we dereference it anyway and are rewarded\n> with a segfault. This should not happen anymore, but it's still wrong to\n> dereference a potential NULL pointer, especially when we do check for\n> NULL pointer in the next code.\n>\n> Reported-by: Luke Diamand \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> v2 added a new test\n\nThis round looks good to me. Passes all tests with/without\nGIT_TEST_SPLIT_INDEX=true\n\n> read-cache.c | 5 +++--\n> split-index.c | 34 ++++++++++++++++++----------------\n> t/t1700-split-index.sh | 18 ++++++++++++++++++\n> 3 files changed, 39 insertions(+), 18 deletions(-)\n>\n> diff --git a/read-cache.c b/read-cache.c\n> index 8f644f68b4..d140b44f8f 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -2520,7 +2520,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n> \t\treturn err;\n>\n> \t/* Write extension data here */\n> -\tif (!strip_extensions && istate->split_index) {\n> +\tif (!strip_extensions && istate->split_index &&\n> +\t !is_null_oid(&istate->split_index->base_oid)) {\n> \t\tstruct strbuf sb = STRBUF_INIT;\n>\n> \t\terr = write_link_extension(&sb, istate) < 0 ||\n\nHowever, it looks like you based this on a pre-2.20.0 version of\ngit. This conflicts with read-cache.c earlier than 3b1d9e045e (\"eoie:\nadd End of Index Entry (EOIE) extension\", 2018-10-10).\n\nI fixed that manually, and pushed it out to github.com/avar/git.git\nduy-read-cache-null-split-index-fix, that's what I've tested on top of\ncurrent \"master\".\n"}]}