[PATCH v7 3/4] robocopy: Add argument parser and basic copy logic

Zebediah Figura zfigura at codeweavers.com
Mon Oct 18 12:08:38 CDT 2021


On 10/16/21 6:13 AM, Florian Eder wrote:
> Parses path arguments as source, destination and files to include,
> reads all files in the source folder that match any of the files to include
> and copies them to the destination, creating necessary folders in the process
> 
> Signed-off-by: Florian Eder <others.meder at gmail.com>
> ---
> v7: Merged patches 3 and 5, removed all messaging / error outputs and
> made copy function non-recursive for now, until the /S switch is implemented
> ---
>   programs/robocopy/Makefile.in      |   1 +
>   programs/robocopy/main.c           | 196 ++++++++++++++++++++++++++++-
>   programs/robocopy/tests/robocopy.c |  38 +++---
>   3 files changed, 214 insertions(+), 21 deletions(-)
> 
> diff --git a/programs/robocopy/Makefile.in b/programs/robocopy/Makefile.in
> index f5d9dff6e4d..d1f5db665df 100644
> --- a/programs/robocopy/Makefile.in
> +++ b/programs/robocopy/Makefile.in
> @@ -1,6 +1,7 @@
>   MODULE    = robocopy.exe
>   
>   EXTRADLLFLAGS = -mconsole -municode
> +IMPORTS   = kernelbase shlwapi
>   
>   C_SRCS = \
>   	main.c
> diff --git a/programs/robocopy/main.c b/programs/robocopy/main.c
> index 320de52f2d5..6e19170712d 100644
> --- a/programs/robocopy/main.c
> +++ b/programs/robocopy/main.c
> @@ -21,9 +21,201 @@ WINE_DEFAULT_DEBUG_CHANNEL(robocopy);
>   
>   #define WIN32_LEAN_AND_MEAN
>   #include <windows.h>
> +#include <stdlib.h>
> +#include <pathcch.h>
> +#include <shlwapi.h>
> +#include <wine/list.h>
> +
> +struct path_array
> +{
> +    UINT size;
> +    WCHAR *array[1];
> +};
> +
> +struct path
> +{
> +    struct list entry;
> +    WCHAR *name;
> +};
> +
> +struct robocopy_options
> +{
> +    WCHAR *destination;
> +    WCHAR *source;
> +    struct path_array *files;
> +};
> +
> +#define ROBOCOPY_SUCCESS_FILES_COPIED        1
> +#define ROBOCOPY_ERROR_NO_FILES_COPIED        16
> +
> +struct robocopy_options options;
> +
> +static WCHAR *get_absolute_path(const WCHAR *path)
> +{
> +    DWORD size;
> +    WCHAR *absolute_path;
> +
> +    size = GetFullPathNameW(path, 0, NULL, NULL) + 2;
> +    absolute_path = calloc(size, sizeof(WCHAR));
> +    GetFullPathNameW(path, size, absolute_path, NULL);
> +    PathCchAddBackslashEx(absolute_path, size, NULL, NULL);
> +    return absolute_path;
> +}

Do we need to convert paths to absolute before copying them?

(Do we even need to convert paths to absolute before writing output, and 
if so, should that happen in this patch?)

> +
> +static BOOL path_in_array(WCHAR *name, struct path_array *names)

Both of these arguments could be const. There's a few other cases below.

> +{
> +    int i;
> +    for (i = 0; i < names->size; i++)
> +    {
> +        if (PathMatchSpecW(name, names->array[i])) return TRUE;
> +    }
> +    return FALSE;
> +}
> +
> +static BOOL create_directory_path(WCHAR *path)
> +{
> +    WCHAR *pointer, *current_folder;
> +    current_folder = calloc(wcslen(path) + 1, sizeof(WCHAR));
> +    pointer = wcschr(path, L'\\');
> +    while (pointer != NULL)
> +    {
> +        if (!lstrcpynW(current_folder, path, pointer - path + 2)) return FALSE;
> +        if (!CreateDirectoryW(current_folder, NULL) && GetLastError() != ERROR_ALREADY_EXISTS) return FALSE;
> +        pointer = wcschr(pointer + 1, L'\\');
> +    }
> +    return TRUE;
> +}
> +
> +static void append_matching_directory_content(struct list *paths, WCHAR *search_path, WCHAR *root, struct path_array *file_names)
> +{
> +    WIN32_FIND_DATAW entry_data;
> +    HANDLE handle;
> +    WCHAR *current_absolute_path;
> +    struct path *new_path;
> +
> +    handle = FindFirstFileExW(search_path, FindExInfoStandard, &entry_data, FindExSearchNameMatch, NULL, 0);
> +    if (handle != INVALID_HANDLE_VALUE)

You could return early here and save a level of indentation.

> +    {
> +        do
> +        {
> +            if (!wcscmp(L".", entry_data.cFileName) || !wcscmp(L"..", entry_data.cFileName)) continue;
> +
> +            PathAllocCombine(root, entry_data.cFileName,
> +                             PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS,
> +                             &current_absolute_path);

Have you considered adding a helper for this PathAllocCombine() call? 
It's only one function, granted, but it's done many times here, and it 
does take up several lines.

> +
> +            if ((!PathIsDirectoryW(current_absolute_path) && path_in_array(entry_data.cFileName, file_names)) ||
> +                (PathIsDirectoryW(current_absolute_path)))

This could be simplified to "PathIsDirectory || path_in_array(...)".

> +            {
> +                new_path = calloc(1, sizeof(struct path));
> +                new_path->name = wcsdup(entry_data.cFileName);
> +                list_add_tail(paths, &new_path->entry);
> +            }
> +        }
> +        while (FindNextFileW(handle, &entry_data));
> +    }
> +}



More information about the wine-devel mailing list