Commit cccacac555ed3937204f8dfd397bc92e499d2c42

Russell Belfer 2012-11-14T22:41:51

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.

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
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
diff --git a/src/checkout.c b/src/checkout.c
index 0d14e26..3c00786 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 5eebc50..7f023bf 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 f6f2e23..6980c36 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 d99864d..ee61c2d 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 557f4f3..7359e4e 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 396af7c..0659a5d 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 3bd1549..f62863a 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 0000000..cbfc66c
--- /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);
+}
+