[PATCH v8 2/3] robocopy: Add argument parser and basic copy logic

[Zebediah Figura (she/her)]

On 10/31/21 15:34, 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>
> ---
> v8: Made some arguments constant, added helper function to replace / remove duplicates of very long flags for PathAllocCombine and
> simplified logic in append_matching_directory_content
> 
> This patch still converts both source and destination path to absolute paths, as doing otherwise would break the already existing
> support for paths > MAX_PATH. It would also not really save many LoCs / much logic, as it would still be required to make sure the
> paths both end with a backslash, to simplify the copy logic (as we then can just append the relative paths).

Right, thanks, that makes sense.

Just a few more nitpicks below...

> +struct robocopy_options options;

Missing "static".

> +static void combine_path(const WCHAR *path1, const WCHAR *path2, WCHAR **out)
> +{
> +    PathAllocCombine(path1, path2,
> +                     PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS,
> +                     out);
> +}

You might consider making the combined path the return value. Food for 
thought.

> +
> +static void append_matching_directory_content(struct list *paths, const WCHAR *search_path, const WCHAR *root, struct path_array *file_names)

"file_names" could be const. This line is also kind of long—we don't 
really have a hard limit, but 120 characters is pushing it.

> +{
> +    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) return;
> +    do
> +    {
> +        if (!wcscmp(L".", entry_data.cFileName) || !wcscmp(L"..", entry_data.cFileName)) continue;
> +
> +        combine_path(root, entry_data.cFileName, &current_absolute_path);
> +
> +        if (PathIsDirectoryW(current_absolute_path) || path_in_array(entry_data.cFileName, file_names))
> +        {
> +            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));
> +}
> +
> +static void get_file_paths_in_folder(const WCHAR *root, struct list *paths)
> +{
> +    struct path *new_path;
> +    WCHAR *current_search_path;
> +
> +    list_init(paths);

This list is initialized both here and in the caller. Not sure which 
makes more sense given following patches, but once is enough ;-)

> +    new_path = calloc(1, sizeof(struct path));
> +    new_path->name = calloc(2, sizeof(WCHAR));
> +    list_add_tail(paths, &new_path->entry);
> +
> +    combine_path(root, L"*", &current_search_path);
> +    append_matching_directory_content(paths, current_search_path, root, options.files);
> +}