{"thread":{"id":"45792","subject":"[PATCH] submodule_init: die cleanly on submodules without url defined","startedAt":"2017-04-25T00:57:58Z","lastAt":"2017-04-25T15:54:20Z","messageCount":2,"participants":["Jeff King","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"317748","messageId":"20170425005746.tulvexabonlexah3@sigill.intra.peff.net","threadId":"45792","inReplyTo":null,"subject":"[PATCH] submodule_init: die cleanly on submodules without url defined","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-25T00:57:47Z","receivedAt":"2017-04-25T00:57:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we init a submodule, we try to die when it has no URL\ndefined:\n\n  url = xstrdup(sub->url);\n  if (!url)\n\t  die(...);\n\nBut that's clearly nonsense. xstrdup() will never return\nNULL, and if sub->url is NULL, we'll segfault.\n\nThese two bits of code need to be flipped, so we check\nsub->url before looking at it.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe bug is from the original submodule--helper, so it first appeared in\nv2.9.x.\n\n builtin/submodule--helper.c | 6 +++---\n t/t7400-submodule-basic.sh  | 8 ++++++++\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 36e423182..566a5b6a6 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -376,12 +376,12 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n \tif (git_config_get_string(sb.buf, &url)) {\n-\t\turl = xstrdup(sub->url);\n-\n-\t\tif (!url)\n+\t\tif (!sub->url)\n \t\t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n \t\t\t\tdisplaypath);\n \n+\t\turl = xstrdup(sub->url);\n+\n \t\t/* Possibly a url relative to parent */\n \t\tif (starts_with_dot_dot_slash(url) ||\n \t\t    starts_with_dot_slash(url)) {\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex c2706fe47..1b8f1dbd3 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -38,6 +38,14 @@ test_expect_success 'submodule update aborts on missing .gitmodules file' '\n \ttest_i18ngrep \"Submodule path .sub. not initialized\" actual\n '\n \n+test_expect_success 'submodule update aborts on missing gitmodules url' '\n+\ttest_when_finished \"git update-index --remove sub\" &&\n+\tgit update-index --add --cacheinfo 160000,$(git rev-parse HEAD),sub &&\n+\ttest_when_finished \"rm -f .gitmodules\" &&\n+\tgit config -f .gitmodules submodule.s.path sub &&\n+\ttest_must_fail git submodule init\n+'\n+\n test_expect_success 'configuration parsing' '\n \ttest_when_finished \"rm -f .gitmodules\" &&\n \tcat >.gitmodules <<-\\EOF &&\n-- \n2.13.0.rc0.459.g06dc2b676\n"},{"id":"317816","messageId":"CAGZ79kb9hsvrzc=RnhhyJf-tTnr9rTfV-TQWimYFGwU_Kuew9w@mail.gmail.com","threadId":"45792","inReplyTo":"20170425005746.tulvexabonlexah3@sigill.intra.peff.net","subject":"Re: [PATCH] submodule_init: die cleanly on submodules without url defined","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-25T15:52:48Z","receivedAt":"2017-04-25T15:54:20Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Apr 24, 2017 at 5:57 PM, Jeff King <peff@peff.net> wrote:\n> When we init a submodule, we try to die when it has no URL\n> defined:\n>\n>   url = xstrdup(sub->url);\n>   if (!url)\n>           die(...);\n>\n> But that's clearly nonsense. xstrdup() will never return\n> NULL, and if sub->url is NULL, we'll segfault.\n>\n> These two bits of code need to be flipped, so we check\n> sub->url before looking at it.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n\nMakes sense. At the time I assumed xstrdup had a _or_null\nbehavior (i.e. if NULL goes in, it is not duplicated, but rather\nreturn NULL)\n\nThanks,\nStefan\n"}]}