Merge pull request #1071 from arrbee/alternate-fix-strcmp

Win32 fixes for diff/checkout/reset

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;

src/diff.c

@@ -554,15 +554,15 @@ static int diff_list_init_from_iterators(
 	if (!old_iter->ignore_case && !new_iter->ignore_case) {
 		diff->opts.flags &= ~GIT_DIFF_DELTAS_ARE_ICASE;
 
-		diff->strcomp    = strcmp;
-		diff->strncomp   = strncmp;
+		diff->strcomp    = git__strcmp;
+		diff->strncomp   = git__strncmp;
 		diff->pfxcomp    = git__prefixcmp;
 		diff->entrycomp  = git_index_entry__cmp;
 	} else {
 		diff->opts.flags |= GIT_DIFF_DELTAS_ARE_ICASE;
 
-		diff->strcomp    = strcasecmp;
-		diff->strncomp   = strncasecmp;
+		diff->strcomp    = git__strcasecmp;
+		diff->strncomp   = git__strncasecmp;
 		diff->pfxcomp    = git__prefixcmp_icase;
 		diff->entrycomp  = git_index_entry__cmp_icase;
 	}

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) {

src/pathspec.c

@@ -122,11 +122,11 @@ bool git_pathspec_match_path(
 		fnmatch_flags = FNM_CASEFOLD;
 
 	if (casefold) {
-		use_strcmp  = strcasecmp;
-		use_strncmp = strncasecmp;
+		use_strcmp  = git__strcasecmp;
+		use_strncmp = git__strncasecmp;
 	} else {
-		use_strcmp  = strcmp;
-		use_strncmp = strncmp;
+		use_strcmp  = git__strcmp;
+		use_strncmp = git__strncmp;
 	}
 
 	git_vector_foreach(vspec, i, match) {

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

src/util.c

@@ -174,6 +174,36 @@ int git__strtol32(int32_t *result, const char *nptr, const char **endptr, int ba
 	return error;
 }
 
+int git__strcmp(const char *a, const char *b)
+{
+	while (*a && *b && *a == *b)
+		++a, ++b;
+	return (int)(*(const unsigned char *)a) - (int)(*(const unsigned char *)b);
+}
+
+int git__strcasecmp(const char *a, const char *b)
+{
+	while (*a && *b && tolower(*a) == tolower(*b))
+		++a, ++b;
+	return (tolower(*a) - tolower(*b));
+}
+
+int git__strncmp(const char *a, const char *b, size_t sz)
+{
+	while (sz && *a && *b && *a == *b)
+		--sz, ++a, ++b;
+	if (!sz)
+		return 0;
+	return (int)(*(const unsigned char *)a) - (int)(*(const unsigned char *)b);
+}
+
+int git__strncasecmp(const char *a, const char *b, size_t sz)
+{
+	while (sz && *a && *b && tolower(*a) == tolower(*b))
+		--sz, ++a, ++b;
+	return !sz ? 0 : (tolower(*a) - tolower(*b));
+}
+
 void git__strntolower(char *str, size_t len)
 {
 	size_t i;

src/util.h

@@ -134,6 +134,11 @@ extern int git__bsearch(
 
 extern int git__strcmp_cb(const void *a, const void *b);
 
+extern int git__strcmp(const char *a, const char *b);
+extern int git__strcasecmp(const char *a, const char *b);
+extern int git__strncmp(const char *a, const char *b, size_t sz);
+extern int git__strncasecmp(const char *a, const char *b, size_t sz);
+
 typedef struct {
 	short refcount;
 	void *owner;

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

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;
 
-	git__utf8_to_16(fbuf, GIT_WIN_PATH, file_name);
+	flen = 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;
+	return do_lstat(filename, buf, 0);
+}
 
-	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);
-
-	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;
-	}
-
-	if (buffer == NULL) {
-		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))
-		{
-			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 (GetFileAttributesW(buffer_w) == INVALID_FILE_ATTRIBUTES) {
 		buffer = NULL;
 		errno = ENOENT;
 	}
 
-done:
+	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))
+		{
+			git__free(buffer);
+			buffer = NULL;
+		}
+	}
+
+	else if (!WideCharToMultiByte(
+		CP_UTF8, 0, buffer_w, -1, buffer, GIT_PATH_MAX, NULL, NULL))
+		buffer = NULL;
+
 	if (buffer)
 		git_path_mkposix(buffer);
 

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);
 }

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
 

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);
+}
+

tests-clar/reset/hard.c

@@ -19,6 +19,21 @@ void test_reset_hard__cleanup(void)
 	cl_git_sandbox_cleanup();
 }
 
+static int strequal_ignore_eol(const char *exp, const char *str)
+{
+	while (*exp && *str) {
+		if (*exp != *str) {
+			while (*exp == '\r' || *exp == '\n') ++exp;
+			while (*str == '\r' || *str == '\n') ++str;
+			if (*exp != *str)
+				return false;
+		} else {
+			exp++; str++;
+		}
+	}
+	return (!*exp && !*str);
+}
+
 void test_reset_hard__resetting_reverts_modified_files(void)
 {
 	git_buf path = GIT_BUF_INIT, content = GIT_BUF_INIT;
@@ -61,7 +76,7 @@ void test_reset_hard__resetting_reverts_modified_files(void)
 		cl_git_pass(git_buf_joinpath(&path, wd, files[i]));
 		if (after[i]) {
 			cl_git_pass(git_futils_readbuffer(&content, path.ptr));
-			cl_assert_equal_s(after[i], content.ptr);
+			cl_assert(strequal_ignore_eol(after[i], content.ptr));
 		} else {
 			cl_assert(!git_path_exists(path.ptr));
 		}