[PATCH v2 2/6] setupapi: Implement SetupAddToDiskSpaceList.
Nikolay Sivov
nsivov at codeweavers.com
Sun Oct 20 04:21:48 CDT 2019
On 10/19/19 9:55 PM, Vijay Kiran Kamuju wrote:
> From: Michael Müller <michael at fds-team.de>
>
> From: Michael Müller <michael at fds-team.de>
>
> Signed-off-by: Vijay Kiran Kamuju <infyquest at gmail.com>
> ---
> dlls/setupapi/diskspace.c | 137 +++++++++++++++++++++++++--
> dlls/setupapi/tests/diskspace.c | 159 +++++++++++++++++++++++++++++++-
> 2 files changed, 285 insertions(+), 11 deletions(-)
>
> diff --git a/dlls/setupapi/diskspace.c b/dlls/setupapi/diskspace.c
> index 4f047dd2b544..dcee29443d71 100644
> --- a/dlls/setupapi/diskspace.c
> +++ b/dlls/setupapi/diskspace.c
> @@ -48,7 +48,21 @@ struct space_list
> UINT flags;
> };
>
> +static LONGLONG get_file_size(WCHAR *path)
> +{
> + HANDLE file;
> + LARGE_INTEGER size;
> +
> + file = CreateFileW(path, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE,
> + NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> + if (file == INVALID_HANDLE_VALUE) return 0;
>
> + if (!GetFileSizeEx(file, &size))
> + size.QuadPart = 0;
> +
> + CloseHandle(file);
> + return size.QuadPart;
> +}
>
> /***********************************************************************
> * SetupCreateDiskSpaceListW (SETUPAPI.@)
> @@ -280,25 +294,128 @@ BOOL WINAPI SetupDestroyDiskSpaceList(HDSKSPC diskspace)
> }
>
> /***********************************************************************
> -* SetupAddToDiskSpaceListA (SETUPAPI.@)
> +* SetupAddToDiskSpaceListW (SETUPAPI.@)
> */
> -BOOL WINAPI SetupAddToDiskSpaceListA(HDSKSPC diskspace, PCSTR targetfile,
> +BOOL WINAPI SetupAddToDiskSpaceListW(HDSKSPC diskspace, PCWSTR targetfile,
> LONGLONG filesize, UINT operation,
> PVOID reserved1, UINT reserved2)
> {
> - FIXME(": stub\n");
> - SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
> - return FALSE;
> + struct space_list *list = diskspace;
> + struct file_entry *file;
> + WCHAR *fullpathW;
> + BOOL ret = FALSE;
> + DWORD size;
> +
> + TRACE("(%p, %s, %s, %u, %p, %u)\n", diskspace, debugstr_w(targetfile),
> + wine_dbgstr_longlong(filesize), operation, reserved1, reserved2);
> +
> + if (!targetfile)
> + return TRUE;
> +
> + if (!diskspace)
> + {
> + SetLastError(ERROR_INVALID_HANDLE);
> + return FALSE;
> + }
> +
> + if (operation != FILEOP_COPY && operation != FILEOP_DELETE)
> + {
> + SetLastError(ERROR_INVALID_PARAMETER);
> + return FALSE;
> + }
> +
> + size = GetFullPathNameW(targetfile, 0, NULL, NULL);
> + if (!size)
> + {
> + SetLastError(ERROR_INVALID_PARAMETER);
> + return FALSE;
> + }
> +
> + size = (size+1) * sizeof(WCHAR);
> + fullpathW = HeapAlloc(GetProcessHeap(), 0, size);
> +
> + if (!GetFullPathNameW(targetfile, size, fullpathW, NULL))
> + {
> + SetLastError(ERROR_INVALID_PARAMETER);
> + goto done;
> + }
> +
> + if (fullpathW[1] != ':' && fullpathW[2] != '\\')
> + {
> + FIXME("UNC paths not yet supported\n");
> + SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
> + goto done;
> + }
> +
> + LIST_FOR_EACH_ENTRY(file, &list->files, struct file_entry, entry)
> + {
> + if (!lstrcmpiW(file->path, fullpathW))
> + break;
> + }
> +
> + if (&file->entry == &list->files)
> + {
I understand the purpose of this check, but it does not look intuitive. May
> + file = HeapAlloc(GetProcessHeap(), 0, sizeof(*file));
> + if (!file)
> + {
> + SetLastError(ERROR_NOT_ENOUGH_MEMORY);
> + goto done;
> + }
> +
> + file->path = strdupW(fullpathW);
> + if (!file->path)
> + {
> + SetLastError(ERROR_NOT_ENOUGH_MEMORY);
> + HeapFree(GetProcessHeap(), 0, file);
> + goto done;
> + }
> +
> + list_add_tail(&list->files, &file->entry);
> + }
> +
> + file->operation = operation;
> + if (operation == FILEOP_COPY)
> + file->size = filesize;
> + else
> + file->size = 0;
> +
> + if (!(list->flags & SPDSL_IGNORE_DISK))
> + file->size -= get_file_size(fullpathW);
> +
> + ret = TRUE;
> +
> +done:
> + HeapFree(GetProcessHeap(), 0, fullpathW);
> + return ret;
> }
>
> /***********************************************************************
> -* SetupAddToDiskSpaceListW (SETUPAPI.@)
> +* SetupAddToDiskSpaceListA (SETUPAPI.@)
> */
> -BOOL WINAPI SetupAddToDiskSpaceListW(HDSKSPC diskspace, PCWSTR targetfile,
> +BOOL WINAPI SetupAddToDiskSpaceListA(HDSKSPC diskspace, PCSTR targetfile,
> LONGLONG filesize, UINT operation,
> PVOID reserved1, UINT reserved2)
> {
> - FIXME(": stub\n");
> - SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
> - return FALSE;
> + LPWSTR targetfileW = NULL;
> + DWORD len;
> + BOOL ret;
> +
> + if (targetfile)
> + {
> + len = MultiByteToWideChar(CP_ACP, 0, targetfile, -1, NULL, 0);
> +
> + targetfileW = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
> + if (!targetfileW)
> + {
> + SetLastError(ERROR_NOT_ENOUGH_MEMORY);
> + return FALSE;
> + }
> +
> + MultiByteToWideChar(CP_ACP, 0, targetfile, -1, targetfileW, len);
> + }
We have strdupAtoW in the same module that you can use to collapse this
part.
> +
> + ret = SetupAddToDiskSpaceListW(diskspace, targetfileW, filesize,
> + operation, reserved1, reserved2);
> + if (targetfileW) HeapFree(GetProcessHeap(), 0, targetfileW);
Null check is unnecessary.
> + return ret;
> }
> diff --git a/dlls/setupapi/tests/diskspace.c b/dlls/setupapi/tests/diskspace.c
> index 4e87ea905e07..bb8c7b78463b 100644
> --- a/dlls/setupapi/tests/diskspace.c
> +++ b/dlls/setupapi/tests/diskspace.c
> @@ -19,6 +19,7 @@
> */
>
> #include <stdarg.h>
> +#include <stdio.h>
>
> #include "windef.h"
> #include "winbase.h"
> @@ -31,6 +32,16 @@
>
> static BOOL is_win9x;
>
> +static inline const char* debugstr_longlong(ULONGLONG ll)
> +{
> + static char string[17];
> + if (sizeof(ll) > sizeof(unsigned long) && ll >> 32)
> + sprintf(string, "%lx%08lx", (unsigned long)(ll >> 32), (unsigned long)ll);
> + else
> + sprintf(string, "%lx", (unsigned long)ll);
> + return string;
> +}
This could be replaced with wine_dbgstr_longlong() I think.
> +
> static void test_SetupCreateDiskSpaceListA(void)
> {
> HDSKSPC ret;
> @@ -300,11 +311,31 @@ static void test_SetupDuplicateDiskSpaceListW(void)
> ok(SetupDestroyDiskSpaceList(handle), "Expected SetupDestroyDiskSpaceList to succeed\n");
> }
>
> +static LONGLONG get_file_size(char *path)
> +{
> + HANDLE file;
> + LARGE_INTEGER size;
> +
> + file = CreateFileA(path, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE,
> + NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> + if (file == INVALID_HANDLE_VALUE) return 0;
> +
> + if (!GetFileSizeEx(file, &size))
> + size.QuadPart = 0;
> +
> + CloseHandle(file);
> + return size.QuadPart;
> +}
> +
> static void test_SetupQuerySpaceRequiredOnDriveA(void)
> {
> BOOL ret;
> HDSKSPC handle;
> LONGLONG space;
> + char windir[MAX_PATH];
> + char drive[3];
> + char tmp[MAX_PATH];
> + LONGLONG size;
>
> if (is_win9x)
> win_skip("SetupQuerySpaceRequiredOnDriveA crashes with NULL disk space handle on Win9x\n");
> @@ -369,7 +400,7 @@ static void test_SetupQuerySpaceRequiredOnDriveA(void)
> ret = SetupQuerySpaceRequiredOnDriveA(handle, "", NULL, NULL, 0);
> ok(!ret, "Expected SetupQuerySpaceRequiredOnDriveA to return FALSE, got %d\n", ret);
> ok(GetLastError() == ERROR_INVALID_DRIVE,
> - "Expected GetLastError() to return ERROR_INVALID_PARAMETER, got %u\n",
> + "Expected GetLastError() to return ERROR_INVALID_DRIVE, got %u\n",
> GetLastError());
>
> SetLastError(0xdeadbeef);
> @@ -381,6 +412,97 @@ static void test_SetupQuerySpaceRequiredOnDriveA(void)
> "Expected GetLastError() to return ERROR_INVALID_PARAMETER, got %u\n",
> GetLastError());
>
> + GetWindowsDirectoryA(windir, MAX_PATH);
> + drive[0] = windir[0]; drive[1] = windir[1]; drive[2] = 0;
> +
> + snprintf(tmp, MAX_PATH, "%c:\\wine-test-should-not-exist.txt", drive[0]);
> + ret = SetupAddToDiskSpaceListA(handle, tmp, 0x100000, FILEOP_COPY, 0, 0);
> + ok(ret, "Expected SetupAddToDiskSpaceListA to succeed\n");
> +
> + space = 0;
> + ret = SetupQuerySpaceRequiredOnDriveA(handle, drive, &space, NULL, 0);
> + ok(ret, "Expected SetupQuerySpaceRequiredOnDriveA to succeed\n");
> + ok(space == 0x100000, "Expected 0x100000 as required space, got %s\n", debugstr_longlong(space));
> +
> + /* adding the same file again doesn't sum up the size */
> + ret = SetupAddToDiskSpaceListA(handle, tmp, 0x200000, FILEOP_COPY, 0, 0);
> + ok(ret, "Expected SetupAddToDiskSpaceListA to succeed\n");
> +
> + space = 0;
> + ret = SetupQuerySpaceRequiredOnDriveA(handle, drive, &space, NULL, 0);
> + ok(ret, "Expected SetupQuerySpaceRequiredOnDriveA to succeed\n");
> + ok(space == 0x200000, "Expected 0x200000 as required space, got %s\n", debugstr_longlong(space));
> +
> + /* the device doesn't need to exist */
> + snprintf(tmp, MAX_PATH, "F:\\wine-test-should-not-exist.txt");
> + ret = SetupAddToDiskSpaceListA(handle, tmp, 0x200000, FILEOP_COPY, 0, 0);
> + ok(ret, "Expected SetupAddToDiskSpaceListA to succeed\n");
> +
> + ret = SetupQuerySpaceRequiredOnDriveA(handle, "F:", &space, NULL, 0);
> + ok(ret, "Expected SetupQuerySpaceRequiredOnDriveA to succeed\n");
> + ok(space == 0x200000, "Expected 0x100000 as required space, got %s\n", debugstr_longlong(space));
> +
> + ok(SetupDestroyDiskSpaceList(handle),
> + "Expected SetupDestroyDiskSpaceList to succeed\n");
> +
> + handle = SetupCreateDiskSpaceListA(NULL, 0, 0);
> + ok(handle != NULL,
> + "Expected SetupCreateDiskSpaceListA to return a valid handle, got NULL\n");
> +
> + /* the real size is subtracted unless SPDSL_IGNORE_DISK is specified */
> + snprintf(tmp, MAX_PATH, "%s\\regedit.exe", windir);
> +
> + size = get_file_size(tmp);
> + ret = SetupAddToDiskSpaceListA(handle, tmp, size, FILEOP_COPY, 0, 0);
> + ok(ret, "Expected SetupAddToDiskSpaceListA to succeed\n");
> + space = 0;
> + ret = SetupQuerySpaceRequiredOnDriveA(handle, drive, &space, NULL, 0);
> + ok(ret, "Expected SetupQuerySpaceRequiredOnDriveA to succeed\n");
> + ok(space == 0 || broken(space == -0x5000) || broken(space == -0x7000),
> + "Expected 0x0 as required space, got %s\n", debugstr_longlong(space));
> +
> + ret = SetupAddToDiskSpaceListA(handle, tmp, size + 0x100000, FILEOP_COPY, 0, 0);
> + ok(ret, "Expected SetupAddToDiskSpaceListA to succeed\n");
> + ret = SetupQuerySpaceRequiredOnDriveA(handle, drive, &space, NULL, 0);
> + ok(ret, "Expected SetupQuerySpaceRequiredOnDriveA to succeed\n");
> + ok(space == 0x100000 || broken(space == 0xf9000) || broken(space == 0xfb000),
> + "Expected 0x100000 as required space, got %s\n", debugstr_longlong(space));
> +
> + ret = SetupAddToDiskSpaceListA(handle, tmp, size - 0x1000, FILEOP_COPY, 0, 0);
> + ok(ret, "Expected SetupAddToDiskSpaceListA to succeed\n");
> + ret = SetupQuerySpaceRequiredOnDriveA(handle, drive, &space, NULL, 0);
> + ok(ret, "Expected SetupQuerySpaceRequiredOnDriveA to succeed\n");
> + ok(space == -0x1000 || broken(space == -0x6000) || broken(space == -0x8000),
> + "Expected -0x1000 as required space, got %s\n", debugstr_longlong(space));
> +
> + ok(SetupDestroyDiskSpaceList(handle),
> + "Expected SetupDestroyDiskSpaceList to succeed\n");
> +
> + handle = SetupCreateDiskSpaceListA(NULL, 0, 0);
> + ok(handle != NULL,
> + "Expected SetupCreateDiskSpaceListA to return a valid handle, got NULL\n");
> +
> + ret = SetupAddToDiskSpaceListA(handle, tmp, size, FILEOP_DELETE, 0, 0);
> + ok(ret, "Expected SetupAddToDiskSpaceListA to succeed\n");
> + space = 0;
> + ret = SetupQuerySpaceRequiredOnDriveA(handle, drive, &space, NULL, 0);
> + ok(ret, "Expected SetupQuerySpaceRequiredOnDriveA to succeed\n");
> + ok(space <= -size, "Expected space <= -size, got %s\n", debugstr_longlong(space));
> +
> + ok(SetupDestroyDiskSpaceList(handle),
> + "Expected SetupDestroyDiskSpaceList to succeed\n");
> +
> + handle = SetupCreateDiskSpaceListA(NULL, 0, SPDSL_IGNORE_DISK);
> + ok(handle != NULL,
> + "Expected SetupCreateDiskSpaceListA to return a valid handle, got NULL\n");
> +
> + ret = SetupAddToDiskSpaceListA(handle, tmp, size, FILEOP_DELETE, 0, 0);
> + ok(ret, "Expected SetupAddToDiskSpaceListA to succeed\n");
> + space = 0;
> + ret = SetupQuerySpaceRequiredOnDriveA(handle, drive, &space, NULL, 0);
> + ok(ret, "Expected SetupQuerySpaceRequiredOnDriveA to succeed\n");
> + ok(space == 0, "Expected size = 0, got %s\n", debugstr_longlong(space));
> +
> ok(SetupDestroyDiskSpaceList(handle),
> "Expected SetupDestroyDiskSpaceList to succeed\n");
> }
> @@ -472,6 +594,40 @@ static void test_SetupQuerySpaceRequiredOnDriveW(void)
> "Expected SetupDestroyDiskSpaceList to succeed\n");
> }
>
> +static void test_SetupAddToDiskSpaceListA(void)
> +{
> + HDSKSPC handle;
> + BOOL ret;
> +
> + ret = SetupAddToDiskSpaceListA(NULL, "C:\\some-file.dat", 0, FILEOP_COPY, 0, 0);
> + ok(!ret, "Expected SetupAddToDiskSpaceListA to return FALSE, got %d\n", ret);
> + ok(GetLastError() == ERROR_INVALID_HANDLE,
> + "Expected GetLastError() to return ERROR_INVALID_HANDLE, got %u\n", GetLastError());
> +
> + handle = SetupCreateDiskSpaceListA(NULL, 0, 0);
> + ok(handle != NULL,"Expected SetupCreateDiskSpaceListA to return a valid handle\n");
> +
> + ret = SetupAddToDiskSpaceListA(handle, NULL, 0, FILEOP_COPY, 0, 0);
> + ok(ret || broken(!ret) /* >= Vista */, "Expected SetupAddToDiskSpaceListA to succeed\n");
> +
> + ret = SetupAddToDiskSpaceListA(handle, "C:\\some-file.dat", -20, FILEOP_COPY, 0, 0);
> + ok(ret, "Expected SetupAddToDiskSpaceListA to succeed\n");
> +
> + ret = SetupAddToDiskSpaceListA(handle, "C:\\some-file.dat", 0, FILEOP_RENAME, 0, 0);
> + ok(!ret, "Expected SetupAddToDiskSpaceListA to return FALSE\n");
> + ok(GetLastError() == ERROR_INVALID_PARAMETER,
> + "Expected GetLastError() to return ERROR_INVALID_PARAMETER, got %u\n", GetLastError());
> +
> + ret = SetupAddToDiskSpaceListA(handle, NULL, 0, FILEOP_RENAME, 0, 0);
> + ok(ret || broken(!ret) /* >= Vista */, "Expected SetupAddToDiskSpaceListA to succeed\n");
> +
> + ret = SetupAddToDiskSpaceListA(NULL, NULL, 0, FILEOP_RENAME, 0, 0);
> + ok(ret || broken(!ret) /* >= Vista */, "Expected SetupAddToDiskSpaceListA to succeed\n");
> +
> + ok(SetupDestroyDiskSpaceList(handle),
> + "Expected SetupDestroyDiskSpaceList to succeed\n");
> +}
There is a lot of broken() cases, especially concerning that allegedly
vista+ behavior is considered broken, which is all currently relevant
systems.
> +
> START_TEST(diskspace)
> {
> is_win9x = !SetupCreateDiskSpaceListW((void *)0xdeadbeef, 0xdeadbeef, 0) &&
> @@ -482,4 +638,5 @@ START_TEST(diskspace)
> test_SetupDuplicateDiskSpaceListW();
> test_SetupQuerySpaceRequiredOnDriveA();
> test_SetupQuerySpaceRequiredOnDriveW();
> + test_SetupAddToDiskSpaceListA();
> }
More information about the wine-devel
mailing list