Re: [PATCH] submodule: check for NULL return of get_submodule_ref_store()
- From
René Scharfe <l.s.r@web.de>
- Date
- Mar 28, 2018, 21:14 UTC
- Message-ID
- <38570708-e166-0004-878a-2d8442c12b65@web.de>
- In-Reply-To
- <CAPig+cQ_j4OyBjsZHE8ZPBojqD7HhSEb14-CFY9qYfXX+dafpQ@mail.gmail.com>
Am 28.03.2018 um 22:21 schrieb Eric Sunshine:
Show 29 quoted lines
> On Wed, Mar 28, 2018 at 4:08 PM, Stefan Beller <sbeller@google.com> wrote: >> On Wed, Mar 28, 2018 at 11:57 AM, Eric Sunshine <sunshine@sunshineco.com> wrote: >>>> +test_expect_success 'moving the submodule does not break the superproject' ' >>>> + ( >>>> + cd addtest2 && >>>> + >>>> + mv repo repo.bak && >>>> + git submodule status >actual && >>>> + grep -e "^-" -e repo actual && >>>> + >>>> + mv repo.bak repo >>> >>> Should this "move back" be encapsulated in a test_when_finished? >> >> I thought about that, but decided against it for some reason as I was debating >> where to put the test_when_finished. I mostly saw those at the very beginning >> of a test and wondered if it can be called from within a subshell. >> (I'd not want to put it at the beginning but rather adjacent to the move.) > > It looks like test_when_finished() shouldn't be used in a subshell. > However, wouldn't the following be reasonable? > > mv addtest2/repo addtest2/repo.bak && > test_when_finished "mv addtest2/repo.bak addtest2/repo" && > ( > cd addtest2 && > git submodule status >actual && > grep -e "^-" -e repo actual > )
I like this version, except for the grep call -- it accepts either lines starting with a dash or containing "repo". So if status would report just "-" and nothing else then the test would pass.
Show 7 quoted lines
> Or, even simpler: > > mv addtest2/repo addtest2/repo.bak && > test_when_finished "mv addtest2/repo.bak addtest2/repo" && > git -C addtest2 submodule status >actual && > grep -e "^-" -e repo actual >
This looks nicer here in the script, but doesn't test exactly what users type most of the time, I suppose.
So how about this?
-- >8 -- Subject: [PATCH v3] submodule: check for NULL return of get_submodule_ref_store()
If we can't find a ref store for a submodule then assume the latter is not initialized (or was removed). Print a status line accordingly instead of causing a segmentation fault by passing NULL as the first parameter of refs_head_ref().
Reported-by: Jeremy Feusi <jeremy@feusi.co> Reviewed-by: Stefan Beller <sbeller@google.com> Initial-Test-By: Stefan Beller <sbeller@google.com> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Rene Scharfe <l.s.r@web.de> --- builtin/submodule--helper.c | 8 ++++++-- t/t7400-submodule-basic.sh | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 6ba8587b6d..9a0fb5e784 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -654,9 +654,13 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, displaypath); } else if (!(flags & OPT_CACHED)) { struct object_id oid; + struct ref_store *refs = get_submodule_ref_store(path); - if (refs_head_ref(get_submodule_ref_store(path), - handle_submodule_head_ref, &oid)) + if (!refs) { + print_status(flags, '-', path, ce_oid, displaypath); + goto cleanup; + } + if (refs_head_ref(refs, handle_submodule_head_ref, &oid)) die(_("could not resolve HEAD ref inside the " "submodule '%s'"), path); diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index a39e69a3eb..ef1ea8d6b0 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -821,6 +821,21 @@ test_expect_success 'moving the superproject does not break submodules' ' ) ' +test_expect_success 'moving the submodule does not break the superproject' ' + ( + cd addtest2 && + git submodule status + ) >actual && + sed -e "s/^ \([^ ]* repo\) .*/-\1/" <actual >expect && + mv addtest2/repo addtest2/repo.bak && + test_when_finished "mv addtest2/repo.bak addtest2/repo" && + ( + cd addtest2 && + git submodule status + ) >actual && + test_cmp expect actual +' + test_expect_success 'submodule add --name allows to replace a submodule with another at the same path' ' ( cd addtest2 &&
-- 2.16.3