Add POSIX compat lstat() variant for win32

The existing p_lstat implementation on win32 is not quite POSIX
compliant when setting errno to ENOTDIR.  This adds an option to
make is be compliant so that code (such as checkout) that cares
to have separate behavior for ENOTDIR can use it portably.

This also contains a couple of other minor cleanups in the
posix_w32.c implementations to avoid unnecessary work.

8 files changed, 213 insertions, 60 deletions

diff --git a/src/checkout.c b/src/checkout.c
index 0d14e2625..3c0078651 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -282,7 +282,7 @@ static int checkout_confirm_update_blob(
 	if (git_buf_puts(data->path, delta->new_file.path) < 0)
 		return -1;
 
-	if ((error = p_stat(git_buf_cstr(data->path), &st)) < 0) {
+	if ((error = p_lstat_posixly(git_buf_cstr(data->path), &st)) < 0) {
 		if (errno == ENOENT) {
 			if (update_only)
 				action = CHECKOUT_ACTION__NONE;
diff --git a/src/fileops.c b/src/fileops.c
index 5eebc5057..7f023bf69 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -377,7 +377,7 @@ static int futils__rm_first_parent(git_buf *path, const char *ceiling)
 
 		if (!path->size || git__prefixcmp(path->ptr, ceiling) != 0)
 			error = 0;
-		else if (p_lstat(path->ptr, &st) == 0) {
+		else if (p_lstat_posixly(path->ptr, &st) == 0) {
 			if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode))
 				error = p_unlink(path->ptr);
 			else if (!S_ISDIR(st.st_mode))
@@ -397,7 +397,7 @@ static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path)
 	struct stat st;
 	futils__rmdir_data *data = opaque;
 
-	if ((data->error = p_lstat(path->ptr, &st)) < 0) {
+	if ((data->error = p_lstat_posixly(path->ptr, &st)) < 0) {
 		if (errno == ENOENT)
 			data->error = 0;
 		else if (errno == ENOTDIR) {
diff --git a/src/unix/posix.h b/src/unix/posix.h
index f6f2e2353..6980c36f1 100644
--- a/src/unix/posix.h
+++ b/src/unix/posix.h
@@ -23,4 +23,7 @@
 #define p_setenv(n,v,o) setenv(n,v,o)
 #define p_inet_pton(a, b, c) inet_pton(a, b, c)
 
+/* see win32/posix.h for explanation about why this exists */
+#define p_lstat_posixly(p,b) lstat(p,b)
+
 #endif
diff --git a/src/win32/posix.h b/src/win32/posix.h
index d99864d05..ee61c2d05 100644
--- a/src/win32/posix.h
+++ b/src/win32/posix.h
@@ -50,4 +50,12 @@ extern int p_recv(GIT_SOCKET socket, void *buffer, size_t length, int flags);
 extern int p_send(GIT_SOCKET socket, const void *buffer, size_t length, int flags);
 extern int p_inet_pton(int af, const char* src, void* dst);
 
+/* p_lstat is almost but not quite POSIX correct.  Specifically, the use of
+ * ENOTDIR is wrong, in that it does not mean precisely that a non-directory
+ * entry was encountered.  Making it correct is potentially expensive,
+ * however, so this is a separate version of p_lstat to use when correct
+ * POSIX ENOTDIR semantics is required.
+ */
+extern int p_lstat_posixly(const char *filename, struct stat *buf);
+
 #endif
diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c
index 557f4f3bf..7359e4e9f 100644
--- a/src/win32/posix_w32.c
+++ b/src/win32/posix_w32.c
@@ -52,17 +52,33 @@ GIT_INLINE(time_t) filetime_to_time_t(const FILETIME *ft)
 	return (time_t)winTime;
 }
 
-static int do_lstat(const char *file_name, struct stat *buf)
+#define WIN32_IS_WSEP(CH) ((CH) == L'/' || (CH) == L'\\')
+
+static int do_lstat(
+	const char *file_name, struct stat *buf, int posix_enotdir)
 {
 	WIN32_FILE_ATTRIBUTE_DATA fdata;
-	wchar_t fbuf[GIT_WIN_PATH];
+	wchar_t fbuf[GIT_WIN_PATH], lastch;
 	DWORD last_error;
+	int flen;
+
+	flen = git__utf8_to_16(fbuf, GIT_WIN_PATH, file_name);
 
-	git__utf8_to_16(fbuf, GIT_WIN_PATH, file_name);
+	/* truncate trailing slashes */
+	for (; flen > 0; --flen) {
+		lastch = fbuf[flen - 1];
+		if (WIN32_IS_WSEP(lastch))
+			fbuf[flen - 1] = L'\0';
+		else if (lastch != L'\0')
+			break;
+	}
 
 	if (GetFileAttributesExW(fbuf, GetFileExInfoStandard, &fdata)) {
 		int fMode = S_IREAD;
 
+		if (!buf)
+			return 0;
+
 		if (fdata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
 			fMode |= S_IFDIR;
 		else
@@ -84,10 +100,45 @@ static int do_lstat(const char *file_name, struct stat *buf)
 		buf->st_atime = filetime_to_time_t(&(fdata.ftLastAccessTime));
 		buf->st_mtime = filetime_to_time_t(&(fdata.ftLastWriteTime));
 		buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));
+
 		return 0;
 	}
 
 	last_error = GetLastError();
+
+	/* ERROR_PATH_NOT_FOUND can mean either that a parent directory is
+	 * missing or that an expected directory is a regular file.  If we need
+	 * POSIX behavior, then ENOTDIR must only be set for the second case
+	 * (i.e. entry that is not a dir), and the first case should be ENOENT.
+	 */
+
+	if (last_error == ERROR_PATH_NOT_FOUND && posix_enotdir) {
+		/* scan up path until we find an existing item */
+		while (1) {
+			/* remove last directory component */
+			for (--flen; flen > 0 && !WIN32_IS_WSEP(fbuf[flen]); --flen);
+
+			if (flen <= 0) {
+				last_error = ERROR_FILE_NOT_FOUND;
+				break;
+			}
+
+			fbuf[flen] = L'\0';
+
+			if (GetFileAttributesExW(fbuf, GetFileExInfoStandard, &fdata)) {
+				if (fdata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+					last_error = ERROR_FILE_NOT_FOUND;
+				else
+					last_error = ERROR_PATH_NOT_FOUND;
+				break;
+			}
+
+			last_error = GetLastError();
+			if (last_error == ERROR_FILE_NOT_FOUND)
+				break;
+		}
+	}
+
 	if (last_error == ERROR_FILE_NOT_FOUND)
 		errno = ENOENT;
 	else if (last_error == ERROR_PATH_NOT_FOUND)
@@ -96,36 +147,14 @@ static int do_lstat(const char *file_name, struct stat *buf)
 	return -1;
 }
 
-int p_lstat(const char *file_name, struct stat *buf)
+int p_lstat(const char *filename, struct stat *buf)
 {
-	int error;
-	size_t namelen;
-	char *alt_name;
-
-	if (do_lstat(file_name, buf) == 0)
-		return 0;
-
-	/* if file_name ended in a '/', Windows returned ENOENT;
-	 * try again without trailing slashes
-	 */
-	namelen = strlen(file_name);
-	if (namelen && file_name[namelen-1] != '/')
-		return -1;
-
-	while (namelen && file_name[namelen-1] == '/')
-		--namelen;
-
-	if (!namelen)
-		return -1;
-
-	alt_name = git__strndup(file_name, namelen);
-	if (!alt_name)
-		return -1;
-
-	error = do_lstat(alt_name, buf);
+	return do_lstat(filename, buf, 0);
+}
 
-	git__free(alt_name);
-	return error;
+int p_lstat_posixly(const char *filename, struct stat *buf)
+{
+	return do_lstat(filename, buf, 1);
 }
 
 int p_readlink(const char *link, char *target, size_t target_len)
@@ -268,7 +297,7 @@ int p_getcwd(char *buffer_out, size_t size)
 
 int p_stat(const char* path, struct stat* buf)
 {
-	return do_lstat(path, buf);
+	return do_lstat(path, buf, 0);
 }
 
 int p_chdir(const char* path)
@@ -301,46 +330,42 @@ int p_hide_directory__w32(const char *path)
 
 char *p_realpath(const char *orig_path, char *buffer)
 {
-	int ret, buffer_sz = 0;
+	int ret;
 	wchar_t orig_path_w[GIT_WIN_PATH];
 	wchar_t buffer_w[GIT_WIN_PATH];
 
 	git__utf8_to_16(orig_path_w, GIT_WIN_PATH, orig_path);
+
+	/* Implicitly use GetCurrentDirectory which can be a threading issue */
 	ret = GetFullPathNameW(orig_path_w, GIT_WIN_PATH, buffer_w, NULL);
 
 	/* According to MSDN, a return value equals to zero means a failure. */
-	if (ret == 0 || ret > GIT_WIN_PATH) {
+	if (ret == 0 || ret > GIT_WIN_PATH)
 		buffer = NULL;
-		goto done;
+
+	else if (GetFileAttributesW(buffer_w) == INVALID_FILE_ATTRIBUTES) {
+		buffer = NULL;
+		errno = ENOENT;
 	}
 
-	if (buffer == NULL) {
-		buffer_sz = WideCharToMultiByte(CP_UTF8, 0, buffer_w, -1, NULL, 0, NULL, NULL);
+	else if (buffer == NULL) {
+		int buffer_sz = WideCharToMultiByte(
+			CP_UTF8, 0, buffer_w, -1, NULL, 0, NULL, NULL);
 
 		if (!buffer_sz ||
 			!(buffer = (char *)git__malloc(buffer_sz)) ||
-			!WideCharToMultiByte(CP_UTF8, 0, buffer_w, -1, buffer, buffer_sz, NULL, NULL))
+			!WideCharToMultiByte(
+				CP_UTF8, 0, buffer_w, -1, buffer, buffer_sz, NULL, NULL))
 		{
 			git__free(buffer);
 			buffer = NULL;
-			goto done;
-		}
-	} else {
-		if (!WideCharToMultiByte(CP_UTF8, 0, buffer_w, -1, buffer, GIT_PATH_MAX, NULL, NULL)) {
-			buffer = NULL;
-			goto done;
 		}
 	}
 
-	if (!git_path_exists(buffer)) {
-		if (buffer_sz > 0)
-			git__free(buffer);
-
+	else if (!WideCharToMultiByte(
+		CP_UTF8, 0, buffer_w, -1, buffer, GIT_PATH_MAX, NULL, NULL))
 		buffer = NULL;
-		errno = ENOENT;
-	}
 
-done:
 	if (buffer)
 		git_path_mkposix(buffer);
 
diff --git a/src/win32/utf-conv.c b/src/win32/utf-conv.c
index 396af7cad..0659a5d1c 100644
--- a/src/win32/utf-conv.c
+++ b/src/win32/utf-conv.c
@@ -70,12 +70,12 @@ void git__utf8_to_16(wchar_t *dest, size_t length, const char *src)
 }
 #endif
 
-void git__utf8_to_16(wchar_t *dest, size_t length, const char *src)
+int git__utf8_to_16(wchar_t *dest, size_t length, const char *src)
 {
-	MultiByteToWideChar(CP_UTF8, 0, src, -1, dest, (int)length);
+	return MultiByteToWideChar(CP_UTF8, 0, src, -1, dest, (int)length);
 }
 
-void git__utf16_to_8(char *out, const wchar_t *input)
+int git__utf16_to_8(char *out, const wchar_t *input)
 {
-	WideCharToMultiByte(CP_UTF8, 0, input, -1, out, GIT_WIN_PATH, NULL, NULL);
+	return WideCharToMultiByte(CP_UTF8, 0, input, -1, out, GIT_WIN_PATH, NULL, NULL);
 }
diff --git a/src/win32/utf-conv.h b/src/win32/utf-conv.h
index 3bd1549bc..f62863a76 100644
--- a/src/win32/utf-conv.h
+++ b/src/win32/utf-conv.h
@@ -12,8 +12,8 @@
 
 #define GIT_WIN_PATH (260 + 1)
 
-void git__utf8_to_16(wchar_t *dest, size_t length, const char *src);
-void git__utf16_to_8(char *dest, const wchar_t *src);
+int git__utf8_to_16(wchar_t *dest, size_t length, const char *src);
+int git__utf16_to_8(char *dest, const wchar_t *src);
 
 #endif
 
diff --git a/tests-clar/core/stat.c b/tests-clar/core/stat.c
new file mode 100644
index 000000000..cbfc66c32
--- /dev/null
+++ b/tests-clar/core/stat.c
@@ -0,0 +1,117 @@
+#include "clar_libgit2.h"
+#include "fileops.h"
+#include "path.h"
+#include "posix.h"
+
+void test_core_stat__initialize(void)
+{
+	cl_git_pass(git_futils_mkdir("root/d1/d2", NULL, 0755, GIT_MKDIR_PATH));
+	cl_git_mkfile("root/file", "whatever\n");
+	cl_git_mkfile("root/d1/file", "whatever\n");
+}
+
+void test_core_stat__cleanup(void)
+{
+	git_futils_rmdir_r("root", NULL, GIT_RMDIR_REMOVE_FILES);
+}
+
+#ifdef GIT_WIN32
+#define cl_assert_last_error(val) \
+	do { werr = GetLastError(); cl_assert_equal_i((val), (int)werr); } while (0)
+#else
+#define cl_assert_last_error(val)
+#endif
+
+#define cl_assert_error(val) \
+	do { err = errno; cl_assert_equal_i((val), err); } while (0)
+
+void test_core_stat__0(void)
+{
+	struct stat st;
+	int err;
+#ifdef GIT_WIN32
+	DWORD werr;
+#endif
+
+	cl_assert_equal_i(0, p_lstat("root", &st));
+	cl_assert(S_ISDIR(st.st_mode));
+	cl_assert_last_error(0);
+	cl_assert_error(0);
+
+	cl_assert_equal_i(0, p_lstat("root/", &st));
+	cl_assert(S_ISDIR(st.st_mode));
+	cl_assert_last_error(0);
+	cl_assert_error(0);
+
+	cl_assert_equal_i(0, p_lstat("root/file", &st));
+	cl_assert(S_ISREG(st.st_mode));
+	cl_assert_last_error(0);
+	cl_assert_error(0);
+
+	cl_assert_equal_i(0, p_lstat("root/d1", &st));
+	cl_assert(S_ISDIR(st.st_mode));
+	cl_assert_last_error(0);
+	cl_assert_error(0);
+
+	cl_assert_equal_i(0, p_lstat("root/d1/", &st));
+	cl_assert(S_ISDIR(st.st_mode));
+	cl_assert_last_error(0);
+	cl_assert_error(0);
+
+	cl_assert_equal_i(0, p_lstat("root/d1/file", &st));
+	cl_assert(S_ISREG(st.st_mode));
+	cl_assert_last_error(0);
+	cl_assert_error(0);
+
+	cl_assert(p_lstat("root/missing", &st) < 0);
+	cl_assert_last_error(ERROR_FILE_NOT_FOUND);
+	cl_assert_error(ENOENT);
+
+	cl_assert(p_lstat("root/missing/but/could/be/created", &st) < 0);
+	cl_assert_last_error(ERROR_PATH_NOT_FOUND);
+#ifdef GIT_WIN32
+	cl_assert_error(ENOTDIR);
+#else
+	cl_assert_error(ENOENT);
+#endif
+
+	cl_assert(p_lstat_posixly("root/missing/but/could/be/created", &st) < 0);
+	cl_assert_error(ENOENT);
+
+	cl_assert(p_lstat("root/d1/missing", &st) < 0);
+	cl_assert_last_error(ERROR_FILE_NOT_FOUND);
+	cl_assert_error(ENOENT);
+
+	cl_assert(p_lstat("root/d1/missing/deeper/path", &st) < 0);
+	cl_assert_last_error(ERROR_PATH_NOT_FOUND);
+#ifdef GIT_WIN32
+	cl_assert_error(ENOTDIR);
+#else
+	cl_assert_error(ENOENT);
+#endif
+
+	cl_assert(p_lstat_posixly("root/d1/missing/deeper/path", &st) < 0);
+	cl_assert_error(ENOENT);
+
+	cl_assert(p_lstat_posixly("root/d1/file/deeper/path", &st) < 0);
+	cl_assert_error(ENOTDIR);
+
+	cl_assert(p_lstat("root/file/invalid", &st) < 0);
+	cl_assert_error(ENOTDIR);
+
+	cl_assert(p_lstat_posixly("root/file/invalid", &st) < 0);
+	cl_assert_error(ENOTDIR);
+
+	cl_assert(p_lstat("root/file/invalid/deeper_path", &st) < 0);
+	cl_assert_error(ENOTDIR);
+
+	cl_assert(p_lstat_posixly("root/file/invalid/deeper_path", &st) < 0);
+	cl_assert_error(ENOTDIR);
+
+	cl_assert(p_lstat_posixly("root/d1/file/extra", &st) < 0);
+	cl_assert_error(ENOTDIR);
+
+	cl_assert(p_lstat_posixly("root/d1/file/further/invalid/items", &st) < 0);
+	cl_assert_error(ENOTDIR);
+}
+