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

[PATCH 2/2] clone: use computed length in guess_dir_name

From
Jeff King <peff@peff.net>
Date
Aug 5, 2015, 08:39 UTC
Message-ID
<20150805083945.GB28212@sigill.intra.peff.net>
In-Reply-To
<20150805083526.GA22325@sigill.intra.peff.net>

Commit 7e837c6 (clone: simplify string handling in guess_dir_name(), 2015-07-09) changed clone to use strip_suffix instead of hand-rolled pointer manipulation. However, strip_suffix will strip from the end of a NUL-terminated string, and we may have already stripped some characters (like directory separators, or "/.git"). This leads to commands like:

  git clone host:foo.git/
failing to strip the ".git".

We must instead convert our pointer arithmetic into a computed length and feed that to strip_suffix_mem, which will then reduce the length further for us.

It would be nicer if we could drop the pointer manipulation entirely, and just continually strip using strip_suffix. But that doesn't quite work for two reasons:

  1. The early suffixes we're stripping are not constant; we
     need to look for is_dir_sep, which could be one of
     several characters.
  2. Mid-way through the stripping we compute the pointer
     "start", which shows us the beginning of the pathname.
     Which really give us two lengths to work with: the
     offset from the start of the string, and from the start
     of the path. By using pointers for the early part, we
     can just compute the length from "start" when we need
     it.
Signed-off-by: Jeff King <peff@peff.net>
---
I suspect you _could_ clean up this logic further, but I
really wanted to do the minimal fix for the regression.
Especially because Patrick is hopefully going to sweep
through and make it all more robust soon enough. :)
 builtin/clone.c          | 3 ++-
 t/t5603-clone-dirname.sh | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 303a3a7..bf45199 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -174,7 +174,8 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
 	/*
 	 * Strip .{bundle,git}.
 	 */
-	strip_suffix(start, is_bundle ? ".bundle" : ".git" , &len);
+	len = end - start;
+	strip_suffix_mem(start, &len, is_bundle ? ".bundle" : ".git");
 
 	if (is_bare)
 		dir = xstrfmt("%.*s.git", (int)len, start);
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index a0140b9..46725b9 100755
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -46,7 +46,7 @@ test_clone_dir host:foo foo.git bare
 test_clone_dir host:foo.git foo
 test_clone_dir host:foo.git foo.git bare
 test_clone_dir host:foo/.git foo
-test_clone_dir host:foo/.git foo.git bare fail
+test_clone_dir host:foo/.git foo.git bare
 
 # similar, but using ssh URL rather than host:path syntax
 test_clone_dir ssh://host/foo foo
@@ -54,11 +54,11 @@ test_clone_dir ssh://host/foo foo.git bare
 test_clone_dir ssh://host/foo.git foo
 test_clone_dir ssh://host/foo.git foo.git bare
 test_clone_dir ssh://host/foo/.git foo
-test_clone_dir ssh://host/foo/.git foo.git bare fail
+test_clone_dir ssh://host/foo/.git foo.git bare
 
 # we should remove trailing slashes
 test_clone_dir ssh://host/foo/ foo
-test_clone_dir ssh://host/foo.git/ foo fail
+test_clone_dir ssh://host/foo.git/ foo
 test_clone_dir ssh://host/foo/.git/ foo
 
 # omitting the path should default to the hostname
-- 
2.5.0.148.g63828c1
Previous: Jeff KingNext: Sebastian Schuberth
Message 20 of 24 in “clone: Make use of the strip_suffix() helper method”
  1. clone: Make use of the strip_suffix() helper methodSebastian Schuberth, Jul 9, 2015
  2. Jeff KingJul 9, 2015
  3. Sebastian SchuberthJul 9, 2015
  4. clone: Simplify string handling in guess_dir_name()Sebastian Schuberth, Jul 9, 2015
  5. Junio C HamanoJul 9, 2015
  6. Sebastian SchuberthJul 9, 2015
  7. clone: Simplify string handling in guess_dir_name()Sebastian Schuberth, Jul 9, 2015
  8. clone: simplify string handling in guess_dir_name()Sebastian Schuberth, Jul 9, 2015
  9. Junio C HamanoJul 9, 2015
  10. Sebastian SchuberthJul 9, 2015
  11. Lukas FleischerAug 4, 2015
  12. Sebastian SchuberthAug 4, 2015
  13. Jeff KingAug 4, 2015
  14. Patrick SteinhardtAug 5, 2015
  15. Jeff KingAug 5, 2015
  16. Patrick SteinhardtAug 5, 2015
  17. Jeff KingAug 5, 2015
  18. 0/2 fix clone guess_dir_name regression in v2.4.8Jeff King, Aug 5, 2015
  19. 1/2 clone: add tests for output directoryJeff King, Aug 5, 2015
  20. 2/2 clone: use computed length in guess_dir_nameJeff King, Aug 5, 2015
  21. Sebastian SchuberthAug 5, 2015
  22. Junio C HamanoAug 5, 2015
  23. Jeff KingAug 5, 2015
  24. Junio C HamanoJul 9, 2015

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.