[PATCH 2/2] fsutil: Add support for creating hardlinks
Zebediah Figura
zfigura at codeweavers.com
Tue Aug 4 10:25:48 CDT 2020
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 at fds-team.de>
>
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=22749
> Signed-off-by: Vijay Kiran Kamuju <infyquest at gmail.com>
> Signed-off-by: Arkadiusz Hiler <ahiler at 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();
> +}
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200804/bda82318/attachment.sig>
More information about the wine-devel
mailing list