From: Michael Müller michael@fds-team.de
Fsutil doesn't do anything yet other than printing out usage message. This patch puts some thing in place in preparation for the implementation of hardlink subcommand.
Signed-off-by: Vijay Kiran Kamuju infyquest@gmail.com Signed-off-by: Arkadiusz Hiler ahiler@codeweavers.com --- programs/fsutil/Makefile.in | 4 +++ programs/fsutil/fsutil.mc | 27 ++++++++++++++++ programs/fsutil/main.c | 63 ++++++++++++++++++++++++++++++++++--- programs/fsutil/resources.h | 21 +++++++++++++ 4 files changed, 110 insertions(+), 5 deletions(-) create mode 100644 programs/fsutil/fsutil.mc create mode 100644 programs/fsutil/resources.h
diff --git a/programs/fsutil/Makefile.in b/programs/fsutil/Makefile.in index 64307e83ac..fe736bad5d 100644 --- a/programs/fsutil/Makefile.in +++ b/programs/fsutil/Makefile.in @@ -1,6 +1,10 @@ MODULE = fsutil.exe +IMPORTS = user32
EXTRADLLFLAGS = -mconsole -municode -mno-cygwin
C_SRCS = \ main.c + +MC_SRCS = \ + fsutil.mc diff --git a/programs/fsutil/fsutil.mc b/programs/fsutil/fsutil.mc new file mode 100644 index 0000000000..54c801cb2b --- /dev/null +++ b/programs/fsutil/fsutil.mc @@ -0,0 +1,27 @@ +; +; Copyright 2020 Arkadiusz Hiler for CodeWeavers +; +; This library is free software; you can redistribute it and/or +; modify it under the terms of the GNU Lesser General Public +; License as published by the Free Software Foundation; either +; version 2.1 of the License, or (at your option) any later version. +; +; This library is distributed in the hope that it will be useful, +; but WITHOUT ANY WARRANTY; without even the implied warranty of +; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +; Lesser General Public License for more details. +; +; You should have received a copy of the GNU Lesser General Public +; License along with this library; if not, write to the Free Software +; Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA +; + +LanguageNames=(ENU=0x409:fsutil) + +MessageId=101 +SymbolicName=STRING_USAGE +Language=ENU +- Supported Commands - + +[NONE] +. diff --git a/programs/fsutil/main.c b/programs/fsutil/main.c index eb4e341297..1d61edab75 100644 --- a/programs/fsutil/main.c +++ b/programs/fsutil/main.c @@ -1,5 +1,6 @@ /* * Copyright 2016 Austin English + * Copyright 2016 Michael Müller * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -16,18 +17,70 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */
+#include <windows.h> + #include "wine/debug.h" +#include "resources.h"
WINE_DEFAULT_DEBUG_CHANNEL(fsutil);
+static void output_write(const WCHAR *str, DWORD wlen) +{ + DWORD count, ret; + + ret = WriteConsoleW(GetStdHandle(STD_OUTPUT_HANDLE), str, wlen, &count, NULL); + if (!ret) + { + DWORD len; + char *msgA; + + /* On Windows WriteConsoleW() fails if the output is redirected. So fall + * back to WriteFile(), assuming the console encoding is still the right + * one in that case. + */ + len = WideCharToMultiByte(GetConsoleOutputCP(), 0, str, wlen, NULL, 0, NULL, NULL); + msgA = HeapAlloc(GetProcessHeap(), 0, len * sizeof(char)); + if (!msgA) return; + + WideCharToMultiByte(GetConsoleOutputCP(), 0, str, wlen, msgA, len, NULL, NULL); + WriteFile(GetStdHandle(STD_OUTPUT_HANDLE), msgA, len, &count, FALSE); + HeapFree(GetProcessHeap(), 0, msgA); + } +} + +static int WINAPIV output_string(int msg, ...) +{ + WCHAR out[8192]; + __ms_va_list arguments; + int len; + + __ms_va_start(arguments, msg); + len = FormatMessageW(FORMAT_MESSAGE_FROM_HMODULE, NULL, msg, 0, out, ARRAY_SIZE(out), &arguments); + __ms_va_end(arguments); + + if (len == 0 && GetLastError() != NO_ERROR) + WINE_FIXME("Could not format string: le=%u, msg=%d\n", GetLastError(), msg); + else + output_write(out, len); + + return 0; +} + int __cdecl wmain(int argc, WCHAR *argv[]) { - int i; + int i, ret = 0;
- WINE_FIXME("stub:"); + TRACE("Command line:"); for (i = 0; i < argc; i++) - WINE_FIXME(" %s", wine_dbgstr_w(argv[i])); - WINE_FIXME("\n"); + TRACE(" %s", debugstr_w(argv[i])); + TRACE("\n");
- return 0; + if (argc > 1) + { + FIXME("unsupported command %s\n", debugstr_w(argv[1])); + ret = 1; + } + + output_string(STRING_USAGE); + return ret; } diff --git a/programs/fsutil/resources.h b/programs/fsutil/resources.h new file mode 100644 index 0000000000..36b0ffc35f --- /dev/null +++ b/programs/fsutil/resources.h @@ -0,0 +1,21 @@ +/* + * Copyright 2016 Michael Müller + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <windef.h> + +#define STRING_USAGE 101
From: Micheal Müller michael@fds-team.de
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=22749 Signed-off-by: Vijay Kiran Kamuju infyquest@gmail.com Signed-off-by: Arkadiusz Hiler ahiler@codeweavers.com --- configure | 1 + configure.ac | 1 + programs/fsutil/fsutil.mc | 14 +++- programs/fsutil/main.c | 57 ++++++++++++++++- programs/fsutil/resources.h | 2 + programs/fsutil/tests/Makefile.in | 5 ++ programs/fsutil/tests/fsutil.c | 102 ++++++++++++++++++++++++++++++ 7 files changed, 179 insertions(+), 3 deletions(-) create mode 100644 programs/fsutil/tests/Makefile.in create mode 100644 programs/fsutil/tests/fsutil.c
diff --git a/configure b/configure index 244b66545e..7155852589 100755 --- a/configure +++ b/configure @@ -21417,6 +21417,7 @@ wine_fn_config_makefile programs/find enable_find wine_fn_config_makefile programs/find/tests enable_tests wine_fn_config_makefile programs/findstr enable_findstr wine_fn_config_makefile programs/fsutil enable_fsutil +wine_fn_config_makefile programs/fsutil/tests enable_tests wine_fn_config_makefile programs/hh enable_hh wine_fn_config_makefile programs/hostname enable_hostname wine_fn_config_makefile programs/icacls enable_icacls diff --git a/configure.ac b/configure.ac index 47d0471169..8e60ea315b 100644 --- a/configure.ac +++ b/configure.ac @@ -3962,6 +3962,7 @@ WINE_CONFIG_MAKEFILE(programs/find) WINE_CONFIG_MAKEFILE(programs/find/tests) WINE_CONFIG_MAKEFILE(programs/findstr) WINE_CONFIG_MAKEFILE(programs/fsutil) +WINE_CONFIG_MAKEFILE(programs/fsutil/tests) WINE_CONFIG_MAKEFILE(programs/hh) WINE_CONFIG_MAKEFILE(programs/hostname) WINE_CONFIG_MAKEFILE(programs/icacls) diff --git a/programs/fsutil/fsutil.mc b/programs/fsutil/fsutil.mc index 54c801cb2b..e904bb8644 100644 --- a/programs/fsutil/fsutil.mc +++ b/programs/fsutil/fsutil.mc @@ -23,5 +23,17 @@ SymbolicName=STRING_USAGE Language=ENU - Supported Commands -
-[NONE] +hardlink hardlink management +. +MessageId=102 +SymbolicName=STRING_HARDLINK_USAGE +Language=ENU +- Hardlink - Supported Commands - + +create create a hardlink +. +MessageId=103 +SymbolicName=STRING_HARDLINK_CREATE_USAGE +Language=ENU +Syntax: fsutil hardlink create <new> <existing> . diff --git a/programs/fsutil/main.c b/programs/fsutil/main.c index 1d61edab75..0357d50d23 100644 --- a/programs/fsutil/main.c +++ b/programs/fsutil/main.c @@ -66,6 +66,54 @@ static int WINAPIV output_string(int msg, ...) return 0; }
+static BOOL output_error_string(DWORD error) +{ + LPWSTR pBuffer; + if (FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_ALLOCATE_BUFFER, + NULL, error, 0, (LPWSTR)&pBuffer, 0, NULL)) + { + output_write(pBuffer, lstrlenW(pBuffer)); + LocalFree(pBuffer); + return TRUE; + } + return FALSE; +} + +static int create_hardlink(int argc, WCHAR *argv[]) +{ + if (argc != 5) + { + output_string(STRING_HARDLINK_CREATE_USAGE); + return 1; + } + + if (CreateHardLinkW(argv[3], argv[4], NULL)) + return 0; + + output_error_string(GetLastError()); + return 1; +} + +static int hardlink(int argc, WCHAR *argv[]) +{ + int ret; + + if (argc > 2) + { + if (!_wcsicmp(argv[2], L"create")) + return create_hardlink(argc, argv); + else + { + FIXME("unsupported parameter %s\n", debugstr_w(argv[2])); + ret = 1; + } + } + + output_string(STRING_HARDLINK_USAGE); + return ret; +} + int __cdecl wmain(int argc, WCHAR *argv[]) { int i, ret = 0; @@ -77,8 +125,13 @@ int __cdecl wmain(int argc, WCHAR *argv[])
if (argc > 1) { - FIXME("unsupported command %s\n", debugstr_w(argv[1])); - ret = 1; + if (!wcsicmp(argv[1], L"hardlink")) + return hardlink(argc, argv); + else + { + FIXME("unsupported command %s\n", debugstr_w(argv[1])); + ret = 1; + } }
output_string(STRING_USAGE); diff --git a/programs/fsutil/resources.h b/programs/fsutil/resources.h index 36b0ffc35f..56280d958a 100644 --- a/programs/fsutil/resources.h +++ b/programs/fsutil/resources.h @@ -19,3 +19,5 @@ #include <windef.h>
#define STRING_USAGE 101 +#define STRING_HARDLINK_USAGE 102 +#define STRING_HARDLINK_CREATE_USAGE 103 diff --git a/programs/fsutil/tests/Makefile.in b/programs/fsutil/tests/Makefile.in new file mode 100644 index 0000000000..5a69e47dc6 --- /dev/null +++ b/programs/fsutil/tests/Makefile.in @@ -0,0 +1,5 @@ +TESTDLL = fsutil.exe +IMPORTS = user32 + +C_SRCS = \ + fsutil.c diff --git a/programs/fsutil/tests/fsutil.c b/programs/fsutil/tests/fsutil.c new file mode 100644 index 0000000000..931c0a0cd6 --- /dev/null +++ b/programs/fsutil/tests/fsutil.c @@ -0,0 +1,102 @@ +/* + * Copyright 2020 Arkadiusz Hiler for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <windows.h> +#include <stdio.h> + +#include "wine/test.h" + +static DWORD runcmd(const char* cmd) +{ + STARTUPINFOA si = { sizeof(STARTUPINFOA) }; + PROCESS_INFORMATION pi; + char* wcmd; + DWORD rc; + + /* Create a writable copy for CreateProcessA() */ + wcmd = HeapAlloc(GetProcessHeap(), 0, strlen(cmd) + 1); + strcpy(wcmd, cmd); + + rc = CreateProcessA(NULL, wcmd, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + HeapFree(GetProcessHeap(), 0, wcmd); + + if (!rc) + return 260; + + rc = WaitForSingleObject(pi.hProcess, 5000); + + if (rc == WAIT_OBJECT_0) + GetExitCodeProcess(pi.hProcess, &rc); + else + TerminateProcess(pi.hProcess, 1); + + CloseHandle(pi.hThread); + CloseHandle(pi.hProcess); + + return rc; +} + +static void test_hardlink(void) +{ + DWORD rc; + HANDLE hfile; + BY_HANDLE_FILE_INFORMATION info; + + hfile = CreateFileA("file", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, NULL); + ok(hfile != INVALID_HANDLE_VALUE, "failed to create a file\n"); + + if (hfile == INVALID_HANDLE_VALUE) + { + skip("skipping hardlink tests\n"); + return; + } + + CloseHandle(hfile); + + rc = runcmd("fsutil hardlink create link file"); + ok(rc == 0, "failed to create a hardlink\n"); + + hfile = CreateFileA("link", GENERIC_READ, 0, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + ok(hfile != INVALID_HANDLE_VALUE, "failed to open the hardlink\n"); + ok(GetFileInformationByHandle(hfile, &info), "faile to get info about hardlink\n"); + CloseHandle(hfile); + + ok(info.nNumberOfLinks == 2, "our link is not a hardlink"); + + rc = runcmd("fsutil hardlink create link file"); + ok(rc != 0, "fsutil didn't complain about already existing destination\n"); + + rc = runcmd("fsutil hardlink create newlink nonexistingfile"); + ok(rc != 0, "fsutil didn't complain about nonexisting source file\n"); + + DeleteFileA("link"); + DeleteFileA("file"); +} + +START_TEST(fsutil) +{ + char tmpdir[MAX_PATH]; + + GetTempPathA(MAX_PATH, tmpdir); + SetCurrentDirectoryA(tmpdir); + trace("%s\n", tmpdir); + + test_hardlink(); +}
Hello Arkadiusz;
This patch looks in relatively good shape to me; I have a few comments and nitpicks inlined.
On 8/4/20 10:05 AM, Arkadiusz Hiler wrote:
From: Micheal Müller michael@fds-team.de
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=22749 Signed-off-by: Vijay Kiran Kamuju infyquest@gmail.com Signed-off-by: Arkadiusz Hiler ahiler@codeweavers.com
configure | 1 + configure.ac | 1 + programs/fsutil/fsutil.mc | 14 +++- programs/fsutil/main.c | 57 ++++++++++++++++- programs/fsutil/resources.h | 2 + programs/fsutil/tests/Makefile.in | 5 ++ programs/fsutil/tests/fsutil.c | 102 ++++++++++++++++++++++++++++++ 7 files changed, 179 insertions(+), 3 deletions(-) create mode 100644 programs/fsutil/tests/Makefile.in create mode 100644 programs/fsutil/tests/fsutil.c
diff --git a/configure b/configure index 244b66545e..7155852589 100755 --- a/configure +++ b/configure @@ -21417,6 +21417,7 @@ wine_fn_config_makefile programs/find enable_find wine_fn_config_makefile programs/find/tests enable_tests wine_fn_config_makefile programs/findstr enable_findstr wine_fn_config_makefile programs/fsutil enable_fsutil +wine_fn_config_makefile programs/fsutil/tests enable_tests wine_fn_config_makefile programs/hh enable_hh wine_fn_config_makefile programs/hostname enable_hostname wine_fn_config_makefile programs/icacls enable_icacls diff --git a/configure.ac b/configure.ac index 47d0471169..8e60ea315b 100644 --- a/configure.ac +++ b/configure.ac @@ -3962,6 +3962,7 @@ WINE_CONFIG_MAKEFILE(programs/find) WINE_CONFIG_MAKEFILE(programs/find/tests) WINE_CONFIG_MAKEFILE(programs/findstr) WINE_CONFIG_MAKEFILE(programs/fsutil) +WINE_CONFIG_MAKEFILE(programs/fsutil/tests) WINE_CONFIG_MAKEFILE(programs/hh) WINE_CONFIG_MAKEFILE(programs/hostname) WINE_CONFIG_MAKEFILE(programs/icacls) diff --git a/programs/fsutil/fsutil.mc b/programs/fsutil/fsutil.mc index 54c801cb2b..e904bb8644 100644 --- a/programs/fsutil/fsutil.mc +++ b/programs/fsutil/fsutil.mc @@ -23,5 +23,17 @@ SymbolicName=STRING_USAGE Language=ENU
- Supported Commands -
-[NONE] +hardlink hardlink management +. +MessageId=102 +SymbolicName=STRING_HARDLINK_USAGE +Language=ENU +- Hardlink - Supported Commands -
+create create a hardlink +. +MessageId=103 +SymbolicName=STRING_HARDLINK_CREATE_USAGE +Language=ENU +Syntax: fsutil hardlink create <new> <existing> . diff --git a/programs/fsutil/main.c b/programs/fsutil/main.c index 1d61edab75..0357d50d23 100644 --- a/programs/fsutil/main.c +++ b/programs/fsutil/main.c @@ -66,6 +66,54 @@ static int WINAPIV output_string(int msg, ...) return 0; }
+static BOOL output_error_string(DWORD error) +{
- LPWSTR pBuffer;
- if (FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM |
FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_ALLOCATE_BUFFER,NULL, error, 0, (LPWSTR)&pBuffer, 0, NULL))- {
output_write(pBuffer, lstrlenW(pBuffer));LocalFree(pBuffer);return TRUE;- }
- return FALSE;
+}
+static int create_hardlink(int argc, WCHAR *argv[]) +{
- if (argc != 5)
- {
output_string(STRING_HARDLINK_CREATE_USAGE);return 1;- }
- if (CreateHardLinkW(argv[3], argv[4], NULL))
return 0;- output_error_string(GetLastError());
- return 1;
+}
+static int hardlink(int argc, WCHAR *argv[]) +{
- int ret;
- if (argc > 2)
- {
if (!_wcsicmp(argv[2], L"create"))
I think we can drop the underscore here; I'm not sure if it appears elsewhere but it's a historical artifact if so.
return create_hardlink(argc, argv);else{FIXME("unsupported parameter %s\n", debugstr_w(argv[2]));ret = 1;}- }
- output_string(STRING_HARDLINK_USAGE);
- return ret;
+}
int __cdecl wmain(int argc, WCHAR *argv[]) { int i, ret = 0; @@ -77,8 +125,13 @@ int __cdecl wmain(int argc, WCHAR *argv[])
if (argc > 1) {
FIXME("unsupported command %s\n", debugstr_w(argv[1]));ret = 1;
if (!wcsicmp(argv[1], L"hardlink"))return hardlink(argc, argv);else{FIXME("unsupported command %s\n", debugstr_w(argv[1]));ret = 1;}}
output_string(STRING_USAGE);
diff --git a/programs/fsutil/resources.h b/programs/fsutil/resources.h index 36b0ffc35f..56280d958a 100644 --- a/programs/fsutil/resources.h +++ b/programs/fsutil/resources.h @@ -19,3 +19,5 @@ #include <windef.h>
#define STRING_USAGE 101 +#define STRING_HARDLINK_USAGE 102 +#define STRING_HARDLINK_CREATE_USAGE 103 diff --git a/programs/fsutil/tests/Makefile.in b/programs/fsutil/tests/Makefile.in new file mode 100644 index 0000000000..5a69e47dc6 --- /dev/null +++ b/programs/fsutil/tests/Makefile.in @@ -0,0 +1,5 @@ +TESTDLL = fsutil.exe +IMPORTS = user32
+C_SRCS = \
- fsutil.c
diff --git a/programs/fsutil/tests/fsutil.c b/programs/fsutil/tests/fsutil.c new file mode 100644 index 0000000000..931c0a0cd6 --- /dev/null +++ b/programs/fsutil/tests/fsutil.c @@ -0,0 +1,102 @@ +/*
- Copyright 2020 Arkadiusz Hiler for CodeWeavers
- This library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
- This library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
- You should have received a copy of the GNU Lesser General Public
- License along with this library; if not, write to the Free Software
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
- */
+#include <windows.h> +#include <stdio.h>
+#include "wine/test.h"
+static DWORD runcmd(const char* cmd) +{
- STARTUPINFOA si = { sizeof(STARTUPINFOA) };
- PROCESS_INFORMATION pi;
- char* wcmd;
- DWORD rc;
- /* Create a writable copy for CreateProcessA() */
- wcmd = HeapAlloc(GetProcessHeap(), 0, strlen(cmd) + 1);
- strcpy(wcmd, cmd);
- rc = CreateProcessA(NULL, wcmd, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi);
- HeapFree(GetProcessHeap(), 0, wcmd);
- if (!rc)
return 260;- rc = WaitForSingleObject(pi.hProcess, 5000);
- if (rc == WAIT_OBJECT_0)
GetExitCodeProcess(pi.hProcess, &rc);- else
TerminateProcess(pi.hProcess, 1);- CloseHandle(pi.hThread);
- CloseHandle(pi.hProcess);
- return rc;
+}
+static void test_hardlink(void) +{
- DWORD rc;
- HANDLE hfile;
- BY_HANDLE_FILE_INFORMATION info;
- hfile = CreateFileA("file", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS,
FILE_ATTRIBUTE_NORMAL, NULL);- ok(hfile != INVALID_HANDLE_VALUE, "failed to create a file\n");
- if (hfile == INVALID_HANDLE_VALUE)
- {
skip("skipping hardlink tests\n");return;- }
I'm personally of the opinion that these kinds of checks aren't particularly useful—if a test fails, that's already a problem and needs to be addressed.
- CloseHandle(hfile);
- rc = runcmd("fsutil hardlink create link file");
- ok(rc == 0, "failed to create a hardlink\n");
- hfile = CreateFileA("link", GENERIC_READ, 0, NULL, OPEN_EXISTING,
FILE_ATTRIBUTE_NORMAL, NULL);- ok(hfile != INVALID_HANDLE_VALUE, "failed to open the hardlink\n");
- ok(GetFileInformationByHandle(hfile, &info), "faile to get info about hardlink\n");
Not particularly important, but printing the output of GetLastError() is often useful in case the test does ever fail. (Note that in that case you'll also have to move GetFileInformationByHandle() to its own line.)
- CloseHandle(hfile);
- ok(info.nNumberOfLinks == 2, "our link is not a hardlink");
- rc = runcmd("fsutil hardlink create link file");
- ok(rc != 0, "fsutil didn't complain about already existing destination\n");
Should we test the exact value of "rc" here?
- rc = runcmd("fsutil hardlink create newlink nonexistingfile");
- ok(rc != 0, "fsutil didn't complain about nonexisting source file\n");
- DeleteFileA("link");
- DeleteFileA("file");
Generally not a bad idea to check for success from DeleteFile, I think.
+}
+START_TEST(fsutil) +{
- char tmpdir[MAX_PATH];
- GetTempPathA(MAX_PATH, tmpdir);
- SetCurrentDirectoryA(tmpdir);
- trace("%s\n", tmpdir);
Is this trace() message doing anything particularly useful?
- test_hardlink();
+}
On Tue, Aug 04, 2020 at 10:25:48AM -0500, Zebediah Figura wrote:
+static void test_hardlink(void) +{
- DWORD rc;
- HANDLE hfile;
- BY_HANDLE_FILE_INFORMATION info;
- hfile = CreateFileA("file", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS,
FILE_ATTRIBUTE_NORMAL, NULL);- ok(hfile != INVALID_HANDLE_VALUE, "failed to create a file\n");
- if (hfile == INVALID_HANDLE_VALUE)
- {
skip("skipping hardlink tests\n");return;- }
I'm personally of the opinion that these kinds of checks aren't particularly useful—if a test fails, that's already a problem and needs to be addressed.
<SNIP>
+START_TEST(fsutil) +{
- char tmpdir[MAX_PATH];
- GetTempPathA(MAX_PATH, tmpdir);
- SetCurrentDirectoryA(tmpdir);
- trace("%s\n", tmpdir);
Is this trace() message doing anything particularly useful?
Those two were borrowed from other tests. I was checking how xcopy and find are tested while looking for inspiration.
One can argue that if we haven't managed to create the source file everything past that is just noise, but I also see the point of making such failures more colorful and noticable :-)
As of that trace - I thought it may be useful for debugging to see where the files are located for manual inspection upon failure.
Anyway, I went with all of your suggestion, as I am still developing my own taste here.
On 8/5/20 3:39 AM, Arkadiusz Hiler wrote:
On Tue, Aug 04, 2020 at 10:25:48AM -0500, Zebediah Figura wrote:
+static void test_hardlink(void) +{
- DWORD rc;
- HANDLE hfile;
- BY_HANDLE_FILE_INFORMATION info;
- hfile = CreateFileA("file", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS,
FILE_ATTRIBUTE_NORMAL, NULL);- ok(hfile != INVALID_HANDLE_VALUE, "failed to create a file\n");
- if (hfile == INVALID_HANDLE_VALUE)
- {
skip("skipping hardlink tests\n");return;- }
I'm personally of the opinion that these kinds of checks aren't particularly useful—if a test fails, that's already a problem and needs to be addressed.
<SNIP> >> +START_TEST(fsutil) >> +{ >> + char tmpdir[MAX_PATH]; >> + >> + GetTempPathA(MAX_PATH, tmpdir); >> + SetCurrentDirectoryA(tmpdir); >> + trace("%s\n", tmpdir); > > Is this trace() message doing anything particularly useful?
Those two were borrowed from other tests. I was checking how xcopy and find are tested while looking for inspiration.
One can argue that if we haven't managed to create the source file everything past that is just noise, but I also see the point of making such failures more colorful and noticable :-)
As of that trace - I thought it may be useful for debugging to see where the files are located for manual inspection upon failure.
Anyway, I went with all of your suggestion, as I am still developing my own taste here.
Sure, and at least the first comment is arguably just my opinion.
If you want to be more verbose about failure to create a file, I'd put that information in the ok() message. Failing that, I'd at least guard the trace() with "if (winetest_debug > 1)".
For some context, we also have problems with tests printing too much information (we have a hard limit of 15 MB per test result hosted on test.winehq.org, and recently we've been running rather close to that limit.)
-
Arkadiusz Hiler -
Zebediah Figura