On 2/28/22 4:40 AM, Tao Klerks via GitGitGadget wrote:
From: Tao Klerks <tao@xxxxxxxxxx>
The mingw_utime implementation in mingw.c does not support
directories. This means that "test-tool chmtime" fails on Windows when
targeting directories. This has previously been noted and sidestepped
by Jeff Hostetler, in "t/helper/test-chmtime: skip directories
on Windows" in the "Builtin FSMonitor Part 2" work.
It would make sense to backdate file and folder changes in untracked
cache tests, to avoid needing to insert explicit delays/pauses in the
tests.
Add support for directory date manipulation in mingw_utime by calling
CreateFileW() explicitly to create the directory handle, and
CloseHandle() to close it.
Signed-off-by: Tao Klerks <tao@xxxxxxxxxx>
---
compat/mingw.c | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 03af369b2b9..2284ea90511 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -964,6 +964,8 @@ int mingw_utime (const char *file_name, const struct utimbuf *times)
int fh, rc;
DWORD attrs;
wchar_t wfilename[MAX_PATH];
+ HANDLE osfilehandle;
I'd be more comfortable initializing this variable to
INVALID_HANDLE_VALUE.
+
if (xutftowcs_path(wfilename, file_name) < 0)
return -1;
@@ -975,9 +977,26 @@ int mingw_utime (const char *file_name, const struct utimbuf *times)
SetFileAttributesW(wfilename, attrs & ~FILE_ATTRIBUTE_READONLY);
}
- if ((fh = _wopen(wfilename, O_RDWR | O_BINARY)) < 0) {
- rc = -1;
- goto revert_attrs;
+ if (attrs & FILE_ATTRIBUTE_DIRECTORY) {
+ fh = 0;
and here initializing fh = -1.
+ osfilehandle = CreateFileW(wfilename,
+ 0x100 /*FILE_WRITE_ATTRIBUTES*/,
+ 0 /*FileShare.None*/,
Is there a reason that you're not using the #define's here?
I assume you ran into a header file problem or something, but
there are other symbols used nearby, so I'm not sure.
+ NULL,
+ OPEN_EXISTING,
+ FILE_FLAG_BACKUP_SEMANTICS,
+ NULL);
+ if (osfilehandle == INVALID_HANDLE_VALUE) {
+ errno = err_win_to_posix(GetLastError());
+ rc = -1;
+ goto revert_attrs;
+ }
+ } else {
+ if ((fh = _wopen(wfilename, O_RDWR | O_BINARY)) < 0) {
+ rc = -1;
+ goto revert_attrs;
+ }
+ osfilehandle = (HANDLE)_get_osfhandle(fh);
}
if (times) {
@@ -987,12 +1006,16 @@ int mingw_utime (const char *file_name, const struct utimbuf *times)
GetSystemTimeAsFileTime(&mft);
aft = mft;
}
- if (!SetFileTime((HANDLE)_get_osfhandle(fh), NULL, &aft, &mft)) {
+ if (!SetFileTime(osfilehandle, NULL, &aft, &mft)) {
errno = EINVAL;
rc = -1;
} else
rc = 0;
- close(fh);
+
+ if (fh)
+ close(fh);
+ else if (osfilehandle)
+ CloseHandle(osfilehandle);
And then this becomes:
if (fh != -1)
close(fh)
else if (osfilehandle != INVALID_HANDLE_VALUE)
Closehandle(osfilehandle);
Which I think makes it more clear that we'll properly close the handle.
revert_attrs:
if (attrs != INVALID_FILE_ATTRIBUTES &&
An alternative solution would be to replace the _wopen() with
a call to CreateFileW() without the backup flag for non-directories
and just convert the whole routine to use HANDLE's rather fd's.
Thanks
Jeff
