Thread (6 messages) flat view 6 messages, 4 authors, 2016-12-19

Re: [PATCH] fix pushing to //server/share/dir paths on Windows

From: Jeff King <hidden>
Date: 2016-12-14 17:30:41

On Tue, Dec 13, 2016 at 10:32:01PM +0100, Johannes Sixt wrote:
normalize_path_copy() is not prepared to keep the double-slash of a
//server/share/dir kind of path, but treats it like a regular POSIX
style path and transforms it to /server/share/dir.

The bug manifests when 'git push //server/share/dir master' is run,
because tmp_objdir_add_as_alternate() uses the path in normalized
form when it registers the quarantine object database via
link_alt_odb_entries(). Needless to say that the directory cannot be
accessed using the wrongly normalized path.
Thanks for digging this up! I had a feeling that the problem was going
to be in the underlying path code, but I didn't want to just pass the
buck without evidence. :)
-	if (is_dir_sep(*src)) {
+	/*
+	 * Handle initial part of absolute path: "/", "C:/", "\\server\share/".
+	 */
+	offset = offset_1st_component(src);
+	if (offset) {
+		/* Convert the trailing separator to '/' on Windows. */
+		memcpy(dst, src, offset - 1);
+		dst += offset - 1;
 		*dst++ = '/';
Hmm. So this is the "change-of-behavior" bit. Would it be reasonable to
write:

  /* Copy initial part of absolute path, converting separators on Windows */
  const char *end = src + offset_1st_component(src);
  while (src < end) {
	  char c = *src++;
	  if (c == '\\')
		  c = '/';
	  *dst++ = c;
  }

? I'm not sure if it's wrong to convert backslashes in that first
component or not (but certainly we were before). I don't think we'd need
is_dir_sep() in that "if()", because we can leave slashes as-is. But
maybe it would make the code easier to read.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help