[pulseaudio-discuss] [PATCH] core: Provide a replacement for scandir on Win32

[Tanu Kaskinen]

Thanks for the patch! It would be good to get this in the upcoming
release. Some complaints below.

On Mon, 2017-07-24 at 16:34 +0200, Vadim Troshchinskiy wrote:
> diff --git a/src/pulsecore/conf-parser.c b/src/pulsecore/conf-parser.c
> index 60345adf..a08ff9e8 100644
> --- a/src/pulsecore/conf-parser.c
> +++ b/src/pulsecore/conf-parser.c
> @@ -218,6 +218,35 @@ finish:
>          fclose(f);
>  
>      if (use_dot_d) {
> +#ifdef OS_IS_WIN32
> +        char *dir_name = pa_sprintf_malloc("%s.d", filename);
> +        char *pattern = pa_sprintf_malloc("%s\\*.*", dir_name);

The scandir branch uses "*.conf" as the pattern, shouldn't you use the
same pattern?

> +        HANDLE fh;
> +        WIN32_FIND_DATA wfd;
> +
> +        fh = FindFirstFile(pattern, &wfd);
> +        if ( fh != INVALID_HANDLE_VALUE ) {

Nitpicking about coding style: too many spaces sprinkled here. Use this
style:

    if (condition) {

> +            do {
> +                if( ! (wfd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ) {

More space problems.

    if (!(flags & flag)) {

> +                    char *filename2 = pa_sprintf_malloc("%s\\%s", dir_name, wfd.cFileName);
> +                    pa_config_parse(filename2, NULL, t, proplist, false, userdata);
> +                    pa_xfree(filename2);
> +                }
> +            } while(FindNextFile(fh, &wfd));

    } while (condition);

> +            FindClose(fh);
> +        } else {
> +            DWORD err = GetLastError();
> +
> +            if ( err == ERROR_PATH_NOT_FOUND ) {
> +                pa_log_debug("%s does not exist, ignoring.", dir_name);

This could also happen if the directory is empty, or if dir_name
exists, but is not a directory. A more accurate log message would be:

"Pattern %s\\*.conf did not match any files, ignoring."

> +            } else {
> +                pa_log_warn("FindFirstFile failed %s with error %d, ignoring", err, pattern);

Did the compiler not complain about this? You are passing an integer to
a %s and a string to a %d.

Does Windows have some API to convert the error code to some human-
readable string? It's frustrating to get an error message that only
provides a number as a hint about what went wrong.

-- 
Tanu

https://www.patreon.com/tanuk