git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Stefan Beller <sbeller@google.com>
To: pclouds@gmail.com
Cc: git@vger.kernel.org, hvoigt@hvoigt.net,
	kaartic.sivaraam@gmail.com, sbeller@google.com
Subject: [PATCH] submodule: fix NULL correctness in renamed broken submodules
Date: Thu, 14 Jun 2018 10:31:07 -0700
Message-ID: <20180614173107.201885-1-sbeller@google.com> (raw)
In-Reply-To: <CACsJy8Ab3HoVWSWOtCBRYcsnnHnpO-2oEfV60f=H15RuzwpWwQ@mail.gmail.com>

When fetching with recursing into submodules, the fetch logic inspects
the superproject which submodules actually need to be fetched. This is
tricky for submodules that were renamed in the fetched range of commits.
This was implemented in c68f8375760 (implement fetching of moved
submodules, 2017-10-16), and this patch fixes a mistake in the logic
there.

When the warning is printed, the `name` might be NULL as
default_name_or_path can return NULL, so fix the warning to use the path
as obtained from the diff machinery, as that is not NULL.

While at it, make sure we only attempt to load the submodule if a git
directory of the submodule is found as default_name_or_path will return
NULL in case the git directory cannot be found. Note that passing NULL
to submodule_from_name is just a semantic error, as submodule_from_name
accepts NULL as a value, but then the return value is not the submodule
that was asked for, but some arbitrary other submodule. (Cf. 'config_from'
in submodule-config.c: "If any parameter except the cache is a NULL
pointer just return the first submodule. Can be used to check whether
there are any submodules parsed.")

Reported-by: Duy Nguyen <pclouds@gmail.com>
Helped-by: Duy Nguyen <pclouds@gmail.com>
Helped-by: Heiko Voigt <hvoigt@hvoigt.net>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 submodule.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/submodule.c b/submodule.c
index 939d6870ecd..0998ea23458 100644
--- a/submodule.c
+++ b/submodule.c
@@ -740,12 +740,14 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,
 		else {
 			name = default_name_or_path(p->two->path);
 			/* make sure name does not collide with existing one */
-			submodule = submodule_from_name(the_repository, commit_oid, name);
+			if (name)
+				submodule = submodule_from_name(the_repository,
+								commit_oid, name);
 			if (submodule) {
 				warning("Submodule in commit %s at path: "
 					"'%s' collides with a submodule named "
 					"the same. Skipping it.",
-					oid_to_hex(commit_oid), name);
+					oid_to_hex(commit_oid), p->two->path);
 				name = NULL;
 			}
 		}
-- 
2.18.0.rc1.244.gcf134e6275-goog


  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-05 15:31 BUG: submodule code prints '(null)' Duy Nguyen
2018-06-06 18:32 ` Kaartic Sivaraam
2018-06-09 11:04 ` Duy Nguyen
2018-06-11 22:56   ` Stefan Beller
2018-06-14 15:15     ` Heiko Voigt
2018-06-14 15:44       ` Duy Nguyen
2018-06-14 17:31         ` Stefan Beller [this message]
2018-06-14 17:37           ` [PATCH] t5526: test recursive submodules when fetching moved submodules Stefan Beller
2018-06-14 20:23             ` Heiko Voigt
2018-06-14 20:24           ` [PATCH] submodule: fix NULL correctness in renamed broken submodules Heiko Voigt
2018-06-17 14:02           ` Kaartic Sivaraam
2018-06-12 17:35   ` BUG: submodule code prints '(null)' Stefan Beller

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180614173107.201885-1-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=hvoigt@hvoigt.net \
    --cc=kaartic.sivaraam@gmail.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox