git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Eric DeCosta via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Jeff Hostetler <git@jeffhostetler.com>,
	Eric DeCosta <edecosta@mathworks.com>,
	Eric DeCosta <edecosta@mathworks.com>
Subject: [PATCH v2] fsmonitor: handle differences between Windows named pipe functions
Date: Mon, 10 Apr 2023 19:46:02 +0000	[thread overview]
Message-ID: <pull.1503.v2.git.1681155963011.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1503.git.1679678090412.gitgitgadget@gmail.com>

From: Eric DeCosta <edecosta@mathworks.com>

The two functions involved with creating and checking for the existance of
the local fsmonitor named pipe, CratedNamedPipeW and WaitNamedPipeW appear
to handle names with leading slashes or backslashes a bit differently.

If a repo resolves to a remote file system with the UNC path of
//some-server/some-dir/some-repo, CreateNamedPipeW accepts this name and
creates this named pipe: \\.\pipe\some-server\some-dir\some-repo

However, when the same UNC path is passed to WaitNamedPipeW, it fails with
ERROR_FILE_NOT_FOUND.

Skipping the leading slash for UNC paths works for both CreateNamedPipeW and
WaitNamedPipeW. Resulting in a named pipe with the same name as above that
WaitNamedPipeW is able to correctly find.

Signed-off-by: Eric DeCosta <edecosta@mathworks.com>
---
    fsmonitor: handle differences between Windows named pipe functions
    
    The two functions involved with creating and checking for the existance
    of the local fsmonitor named pipe, CratedNamedPipeW and WaitNamedPipeW
    appear to handle names with leading slashes or backslashes a bit
    differently.
    
    If a repo resolves to a remote file system with the UNC path of
    //some-server/some-dir/some-repo, CreateNamedPipeW accepts this name and
    creates this named pipe: .\pipe\some-server\some-dir\some-repo
    
    However, when the same UNC path is passed to WaitNamedPipeW, it fails
    with ERROR_FILE_NOT_FOUND.
    
    Skipping the leading slash for UNC paths works for both CreateNamedPipeW
    and WaitNamedPipeW. Resulting in a named pipe with the same name as
    above that WaitNamedPipeW is able to correctly find.
    
    v1 differs from v0:
    
     * Abandon hashing repo path in favor of simpler solution where the
       leading slash is simply dropped for UNC paths

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1503%2Fedecosta-mw%2Ffsmonitor_windows-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1503/edecosta-mw/fsmonitor_windows-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1503

Range-diff vs v1:

 1:  c5307928cd7 ! 1:  b41f9bd2e96 fsmonitor: handle differences between Windows named pipe functions
     @@ Metadata
       ## Commit message ##
          fsmonitor: handle differences between Windows named pipe functions
      
     -    CreateNamedPipeW is perfectly happy accepting pipe names with seemingly
     -    embedded escape charcters (e.g. \b), WaitNamedPipeW is not and incorrectly
     -    returns ERROR_FILE_NOT_FOUND when clearly a named pipe, succesfully created
     -    with CreateNamedPipeW, exists.
     +    The two functions involved with creating and checking for the existance of
     +    the local fsmonitor named pipe, CratedNamedPipeW and WaitNamedPipeW appear
     +    to handle names with leading slashes or backslashes a bit differently.
      
     -    For example, this network path is problemmatic:
     -    \\batfs-sb29-cifs\vmgr\sbs29\my_git_repo
     +    If a repo resolves to a remote file system with the UNC path of
     +    //some-server/some-dir/some-repo, CreateNamedPipeW accepts this name and
     +    creates this named pipe: \\.\pipe\some-server\some-dir\some-repo
      
     -    In order to work around this issue, rather than using the path to the
     -    worktree directly as the name of the pipe, instead use the hash of the
     -    worktree path.
     +    However, when the same UNC path is passed to WaitNamedPipeW, it fails with
     +    ERROR_FILE_NOT_FOUND.
     +
     +    Skipping the leading slash for UNC paths works for both CreateNamedPipeW and
     +    WaitNamedPipeW. Resulting in a named pipe with the same name as above that
     +    WaitNamedPipeW is able to correctly find.
      
          Signed-off-by: Eric DeCosta <edecosta@mathworks.com>
      
       ## compat/simple-ipc/ipc-win32.c ##
     -@@
     - #include "cache.h"
     -+#include "hex.h"
     - #include "simple-ipc.h"
     - #include "strbuf.h"
     - #include "pkt-line.h"
      @@
       static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t alloc)
       {
       	int off = 0;
     --	struct strbuf realpath = STRBUF_INIT;
     --
     --	if (!strbuf_realpath(&realpath, path, 0))
     --		return -1;
     -+	int ret = 0;
     -+	git_SHA_CTX sha1ctx;
     -+	struct strbuf real_path = STRBUF_INIT;
     -+	struct strbuf pipe_name = STRBUF_INIT;
     -+	unsigned char hash[GIT_MAX_RAWSZ];
     ++	int real_off = 0;
     + 	struct strbuf realpath = STRBUF_INIT;
       
     --	off = swprintf(wpath, alloc, L"\\\\.\\pipe\\");
     --	if (xutftowcs(wpath + off, realpath.buf, alloc - off) < 0)
     -+	if (!strbuf_realpath(&real_path, path, 0))
     + 	if (!strbuf_realpath(&realpath, path, 0))
       		return -1;
       
     --	/* Handle drive prefix */
     --	if (wpath[off] && wpath[off + 1] == L':') {
     --		wpath[off + 1] = L'_';
     --		off += 2;
     --	}
     -+	git_SHA1_Init(&sha1ctx);
     -+	git_SHA1_Update(&sha1ctx, real_path.buf, real_path.len);
     -+	git_SHA1_Final(hash, &sha1ctx);
     -+	strbuf_release(&real_path);
     - 
     --	for (; wpath[off]; off++)
     --		if (wpath[off] == L'/')
     --			wpath[off] = L'\\';
     -+	strbuf_addf(&pipe_name, "git-fsmonitor-%s", hash_to_hex(hash));
     -+	off = swprintf(wpath, alloc, L"\\\\.\\pipe\\");
     -+	if (xutftowcs(wpath + off, pipe_name.buf, alloc - off) < 0)
     -+		ret = -1;
     - 
     --	strbuf_release(&realpath);
     --	return 0;
     -+	strbuf_release(&pipe_name);
     -+	return ret;
     - }
     ++	/* UNC Path, skip leading slash */
     ++	if (realpath.buf[0] == '/' && realpath.buf[1] == '/')
     ++		real_off = 1;
     ++
     + 	off = swprintf(wpath, alloc, L"\\\\.\\pipe\\");
     +-	if (xutftowcs(wpath + off, realpath.buf, alloc - off) < 0)
     ++	if (xutftowcs(wpath + off, realpath.buf + real_off, alloc - off) < 0)
     + 		return -1;
       
     - static enum ipc_active_state get_active_state(wchar_t *pipe_path)
     + 	/* Handle drive prefix */


 compat/simple-ipc/ipc-win32.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c
index 997f6144344..632b12e1ab5 100644
--- a/compat/simple-ipc/ipc-win32.c
+++ b/compat/simple-ipc/ipc-win32.c
@@ -19,13 +19,18 @@
 static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t alloc)
 {
 	int off = 0;
+	int real_off = 0;
 	struct strbuf realpath = STRBUF_INIT;
 
 	if (!strbuf_realpath(&realpath, path, 0))
 		return -1;
 
+	/* UNC Path, skip leading slash */
+	if (realpath.buf[0] == '/' && realpath.buf[1] == '/')
+		real_off = 1;
+
 	off = swprintf(wpath, alloc, L"\\\\.\\pipe\\");
-	if (xutftowcs(wpath + off, realpath.buf, alloc - off) < 0)
+	if (xutftowcs(wpath + off, realpath.buf + real_off, alloc - off) < 0)
 		return -1;
 
 	/* Handle drive prefix */

base-commit: f285f68a132109c234d93490671c00218066ace9
-- 
gitgitgadget

  parent reply	other threads:[~2023-04-10 19:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-24 17:14 [PATCH] fsmonitor: handle differences between Windows named pipe functions Eric DeCosta via GitGitGadget
2023-03-27 11:37 ` Johannes Schindelin
2023-03-27 15:02   ` Jeff Hostetler
2023-03-27 16:08     ` Junio C Hamano
2023-04-06 19:08     ` Eric DeCosta
2023-04-07 20:55       ` Jeff Hostetler
2023-04-10 19:46 ` Eric DeCosta via GitGitGadget [this message]
2023-04-22 20:00   ` [PATCH v2] " Eric DeCosta
2023-04-26 20:33     ` Jeff Hostetler
2023-04-27 13:45       ` Jeff Hostetler
2023-05-08 20:30       ` Eric DeCosta
2023-05-15 21:50         ` Jeff Hostetler

Reply instructions:

You may reply publicly 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=pull.1503.v2.git.1681155963011.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=edecosta@mathworks.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).