React to feedback for UTF-8 <-> WCHAR and reparse work
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281
diff --git a/src/path.c b/src/path.c
index b284546..2690cd8 100644
--- a/src/path.c
+++ b/src/path.c
@@ -495,7 +495,7 @@ bool git_path_is_empty_dir(const char *path)
HANDLE hFind = FindFirstFileW(filter_w, &findData);
/* If the find handle was created successfully, then it's a directory */
- if (INVALID_HANDLE_VALUE != hFind) {
+ if (hFind != INVALID_HANDLE_VALUE) {
empty = true;
do {
diff --git a/src/util.h b/src/util.h
index d94463c..6fb2dc0 100644
--- a/src/util.h
+++ b/src/util.h
@@ -22,6 +22,15 @@
#define GIT_DATE_RFC2822_SZ 32
+/**
+ * Return the length of a constant string.
+ * We are aware that `strlen` performs the same task and is usually
+ * optimized away by the compiler, whilst being safer because it returns
+ * valid values when passed a pointer instead of a constant string; however
+ * this macro will transparently work with wide-char and single-char strings.
+ */
+#define CONST_STRLEN(x) ((sizeof(x)/sizeof(x[0])) - 1)
+
/*
* Custom memory allocation wrappers
* that set error code and error message
diff --git a/src/win32/findfile.c b/src/win32/findfile.c
index bd06d92..86d4ef5 100644
--- a/src/win32/findfile.c
+++ b/src/win32/findfile.c
@@ -119,7 +119,7 @@ static int win32_find_git_in_registry(
/* InstallLocation points to the root of the git directory */
if (!RegQueryValueExW(hKey, L"InstallLocation", NULL, &dwType, (LPBYTE)path, &cbData) &&
- REG_SZ == dwType) {
+ dwType == REG_SZ) {
/* Append the suffix */
wcscat(path, subdir);
diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c
index bef65a3..0d070f6 100644
--- a/src/win32/posix_w32.c
+++ b/src/win32/posix_w32.c
@@ -62,7 +62,7 @@ int p_unlink(const char *path)
/* If the file could not be deleted because it was
* read-only, clear the bit and try again */
- if (-1 == error && EACCES == errno) {
+ if (error == -1 && errno == EACCES) {
_wchmod(buf, 0666);
error = _wunlink(buf);
}
@@ -120,7 +120,7 @@ static int readlink_w(
FILE_SHARE_READ | FILE_SHARE_DELETE, NULL, OPEN_EXISTING,
FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS, NULL);
- if (INVALID_HANDLE_VALUE == handle) {
+ if (handle == INVALID_HANDLE_VALUE) {
errno = ENOENT;
return -1;
}
@@ -149,7 +149,7 @@ static int readlink_w(
if (target_len) {
/* The path may need to have a prefix removed. */
- target_len = git_win32__to_dos(target, target_len);
+ target_len = git_win32__canonicalize_path(target, target_len);
/* Need one additional character in the target buffer
* for the terminating NULL. */
@@ -235,7 +235,7 @@ static int lstat_w(
path[path_len] = L'\0';
attrs = GetFileAttributesW(path);
- if (INVALID_FILE_ATTRIBUTES != attrs) {
+ if (attrs != INVALID_FILE_ATTRIBUTES) {
if (!(attrs & FILE_ATTRIBUTE_DIRECTORY))
errno = ENOTDIR;
break;
@@ -393,23 +393,18 @@ static int getfinalpath_w(
hFile = CreateFileW(path, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_DELETE,
NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
- if (hFile == INVALID_HANDLE_VALUE)
+ if (INVALID_HANDLE_VALUE == hFile)
return -1;
/* Call GetFinalPathNameByHandle */
dwChars = pgfp(hFile, dest, GIT_WIN_PATH_UTF16, FILE_NAME_NORMALIZED);
+ CloseHandle(hFile);
- if (!dwChars || dwChars >= GIT_WIN_PATH_UTF16) {
- DWORD error = GetLastError();
- CloseHandle(hFile);
- SetLastError(error);
+ if (!dwChars || dwChars >= GIT_WIN_PATH_UTF16)
return -1;
- }
-
- CloseHandle(hFile);
/* The path may be delivered to us with a prefix; canonicalize */
- return (int)git_win32__to_dos(dest, dwChars);
+ return (int)git_win32__canonicalize_path(dest, dwChars);
}
static int follow_and_lstat_link(git_win32_path path, struct stat* buf)
@@ -473,7 +468,7 @@ int p_rmdir(const char* path)
error = _wrmdir(buf);
- if (-1 == error) {
+ if (error == -1) {
switch (GetLastError()) {
/* _wrmdir() is documented to return EACCES if "A program has an open
* handle to the directory." This sounds like what everybody else calls
@@ -513,7 +508,7 @@ char *p_realpath(const char *orig_path, char *buffer)
}
/* The path must exist. */
- if (INVALID_FILE_ATTRIBUTES == GetFileAttributesW(buffer_w)) {
+ if (GetFileAttributesW(buffer_w) == INVALID_FILE_ATTRIBUTES) {
errno = ENOENT;
return NULL;
}
diff --git a/src/win32/reparse.h b/src/win32/reparse.h
index 4f56ed0..70f9fd6 100644
--- a/src/win32/reparse.h
+++ b/src/win32/reparse.h
@@ -54,4 +54,4 @@ typedef struct _GIT_REPARSE_DATA_BUFFER {
# define FSCTL_SET_REPARSE_POINT 0x000900a4
#endif
-#endif
\ No newline at end of file
+#endif
diff --git a/src/win32/utf-conv.c b/src/win32/utf-conv.c
index fe94701..b9ccfb5 100644
--- a/src/win32/utf-conv.c
+++ b/src/win32/utf-conv.c
@@ -9,7 +9,7 @@
#include "utf-conv.h"
#ifndef WC_ERR_INVALID_CHARS
-#define WC_ERR_INVALID_CHARS 0x80
+# define WC_ERR_INVALID_CHARS 0x80
#endif
GIT_INLINE(DWORD) get_wc_flags(void)
@@ -87,12 +87,8 @@ int git__utf8_to_16_alloc(wchar_t **dest, const char *src)
utf16_size = MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, src, -1, *dest, utf16_size);
if (!utf16_size) {
- /* Don't let git__free stomp on the thread-local last error code,
- * so that the caller can call giterr_set(GITERR_OS, ...) */
- DWORD last_error = GetLastError();
git__free(*dest);
*dest = NULL;
- SetLastError(last_error);
}
/* Subtract 1 from the result to turn 0 into -1 (an error code) and to not count the NULL
@@ -131,12 +127,8 @@ int git__utf16_to_8_alloc(char **dest, const wchar_t *src)
utf8_size = WideCharToMultiByte(CP_UTF8, dwFlags, src, -1, *dest, utf8_size, NULL, NULL);
if (!utf8_size) {
- /* Don't let git__free stomp on the thread-local last error code,
- * so that the caller can call giterr_set(GITERR_OS, ...) */
- DWORD last_error = GetLastError();
git__free(*dest);
*dest = NULL;
- SetLastError(last_error);
}
/* Subtract 1 from the result to turn 0 into -1 (an error code) and to not count the NULL
diff --git a/src/win32/w32_util.c b/src/win32/w32_util.c
index 50b85a3..2e52525 100644
--- a/src/win32/w32_util.c
+++ b/src/win32/w32_util.c
@@ -7,8 +7,6 @@
#include "w32_util.h"
-#define CONST_STRLEN(x) ((sizeof(x)/sizeof(x[0])) - 1)
-
/**
* Creates a FindFirstFile(Ex) filter string from a UTF-8 path.
* The filter string enumerates all items in the directory.
@@ -26,8 +24,10 @@ bool git_win32__findfirstfile_filter(git_win32_path dest, const char *src)
if (len < 0)
return false;
- /* Ensure that the path does not end with a trailing slash --
- * because we're about to add one! */
+ /* Ensure that the path does not end with a trailing slash,
+ * because we're about to add one. Don't rely our trim_end
+ * helper, because we want to remove the backslash even for
+ * drive letter paths, in this case. */
if (len > 0 &&
(dest[len - 1] == L'/' || dest[len - 1] == L'\\')) {
dest[len - 1] = L'\0';
@@ -35,7 +35,7 @@ bool git_win32__findfirstfile_filter(git_win32_path dest, const char *src)
}
/* Ensure we have enough room to add the suffix */
- if ((size_t)len > GIT_WIN_PATH_UTF16 - ARRAY_SIZE(suffix))
+ if ((size_t)len >= GIT_WIN_PATH_UTF16 - CONST_STRLEN(suffix))
return false;
wcscat(dest, suffix);
@@ -59,11 +59,11 @@ int git_win32__sethidden(const char *path)
attrs = GetFileAttributesW(buf);
/* Ensure the path exists */
- if (INVALID_FILE_ATTRIBUTES == attrs)
+ if (attrs == INVALID_FILE_ATTRIBUTES)
return -1;
/* If the item isn't already +H, add the bit */
- if (0 == (attrs & FILE_ATTRIBUTE_HIDDEN) &&
+ if ((attrs & FILE_ATTRIBUTE_HIDDEN) == 0 &&
!SetFileAttributesW(buf, attrs | FILE_ATTRIBUTE_HIDDEN))
return -1;
@@ -85,7 +85,7 @@ size_t git_win32__path_trim_end(wchar_t *str, size_t len)
/* Don't trim backslashes from drive letter paths, which
* are 3 characters long and of the form C:\, D:\, etc. */
- if (3 == len && git_win32__isalpha(str[0]) && str[1] == ':')
+ if (len == 3 && git_win32__isalpha(str[0]) && str[1] == ':')
break;
len--;
@@ -103,7 +103,7 @@ size_t git_win32__path_trim_end(wchar_t *str, size_t len)
* @param path The path which should be converted.
* @return The length of the modified string (<= the input length)
*/
-size_t git_win32__to_dos(wchar_t *str, size_t len)
+size_t git_win32__canonicalize_path(wchar_t *str, size_t len)
{
static const wchar_t dosdevices_prefix[] = L"\\\?\?\\";
static const wchar_t nt_prefix[] = L"\\\\?\\";
@@ -136,4 +136,4 @@ size_t git_win32__to_dos(wchar_t *str, size_t len)
}
return git_win32__path_trim_end(str, len);
-}
\ No newline at end of file
+}
diff --git a/src/win32/w32_util.h b/src/win32/w32_util.h
index acdee3d..a1d388a 100644
--- a/src/win32/w32_util.h
+++ b/src/win32/w32_util.h
@@ -49,6 +49,6 @@ size_t git_win32__path_trim_end(wchar_t *str, size_t len);
* @param path The path which should be converted.
* @return The length of the modified string (<= the input length)
*/
-size_t git_win32__to_dos(wchar_t *str, size_t len);
+size_t git_win32__canonicalize_path(wchar_t *str, size_t len);
#endif
diff --git a/tests/core/link.c b/tests/core/link.c
index 20d2706..1794a38 100644
--- a/tests/core/link.c
+++ b/tests/core/link.c
@@ -137,7 +137,7 @@ static void do_junction(const char *old, const char *new)
reparse_buf->ReparseTag = IO_REPARSE_TAG_MOUNT_POINT;
reparse_buf->MountPointReparseBuffer.SubstituteNameOffset = 0;
- reparse_buf->MountPointReparseBuffer.SubstituteNameLength = subst_byte_len;
+ reparse_buf->MountPointReparseBuffer.SubstituteNameLength = subst_byte_len;
reparse_buf->MountPointReparseBuffer.PrintNameOffset = (USHORT)(subst_byte_len + sizeof(WCHAR));
reparse_buf->MountPointReparseBuffer.PrintNameLength = print_byte_len;
reparse_buf->ReparseDataLength = reparse_buflen - REPARSE_DATA_HEADER_SIZE;