Fixed compilation for Windows UWP

[dmitrykos]

Fixed compilation for Windows UWP by making get_utf8_argv() unimplemented in case of UWP target.

LoadLibrary is unavailable on UWP. The proposal is to leave get_utf8_argv() unimplemented because argv functionality is not needed for UWP anyway. Alternative solution would be to return 1 by get_utf8_argv() in case of UWP.

[update] Changed implementation to actually make get_utf8_argv() working in UWP environment. The arguments are obtained as per MS example.

[ktmf01]

Why is argv functionality not necessary anyway?

[dmitrykos]

@ktmf01 my assumption was that classic UWP app is Window-based.

Thanks to your question I double checked about Console UWP app possibility and actually it already exists (missed that fact) and described in MS docs "Create a Universal Windows Platform console app".

Therefore I updated my PR with changes which read argv in UWP environment, get_utf8_argv() works again.

[ktmf01]

Thanks for providing this patch. It'll take some time for me to review this.

Getting an alternative for LoadLibrary has been on my mind for a while now. Not for UWP (because it has been officially deprecated for a while now) but for other platforms in which msvcrt.dll might not be available. I think maybe Windows for ARM? That would make this patch have a broader effect.

[dmitrykos]

It seems to me msvcrt.dll is always available, on Windows for ARM too, so LoadLibrary approach looks fine. But, if CMake is used for project generation then it could be possible to lookup __argc, __wargv, _wenviron symbols in stdlib.h and use/link them directly, similar to the approach in my PR.

These symbols are a part of MS C library extension - Global variables and this approach can replace or compliment LoadLibrary implementation.

[ktmf01]

Have you tested this code? It segfaults at my end

[dmitrykos]

Have you tested this code? It segfaults at my end

Yes, I tested in VS under the debugger. Have you tried to run some UWP app, do you have a stack trace? I was following this example to create UWP console app.

[ktmf01]

The problem is that WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_APP) is also true on normal Windows desktop apparently, and it seems __wargv is NULL, causing a segfault.

[dmitrykos]

@ktmf01 you are right, there is an additional check missing. I committed the fix, checked in Win32 and UWP environments. Now works fine.

[dmitrykos]

@ktmf01 I decided to squash both commits for better consistency of the proposed change.

[sezero]

This breaks build when winapifamily.h is not available:
src/libFLAC/../share/win_utf8_io/win_utf8_io.c:44:25: error: missing binary operator before token "("

I suggest something like the following. (Also see https://github.com/libsdl-org/SDL/blob/main/include/SDL3/SDL_platform_defines.h#L144-L170)

diff --git a/src/share/win_utf8_io/win_utf8_io.c b/src/share/win_utf8_io/win_utf8_io.c
index 1892174..a98bbfb 100644
--- a/src/share/win_utf8_io/win_utf8_io.c
+++ b/src/share/win_utf8_io/win_utf8_io.c
@@ -38,15 +38,26 @@
 #include "share/win_utf8_io.h"
 
 #define UTF8_BUFFER_SIZE 32768
 
+#if defined(_MSC_VER) && defined(__has_include)
+#if __has_include(<winapifamily.h>)
+#define HAVE_WINAPIFAMILY_H 1
+#else
+#define HAVE_WINAPIFAMILY_H 0
+#endif
+#endif
+
+#if HAVE_WINAPIFAMILY_H
+#include <winapifamily.h>
 /* detect whether it is Windows APP (UWP) or standard Win32 envionment */
 #if defined(WINAPI_FAMILY_PARTITION) &&\
 	WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_APP) && !WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)
 	#define FLAC_WINDOWS_APP 1
 #else
 	#define FLAC_WINDOWS_APP 0
 #endif
+#endif
 
 static int local_vsnprintf(char *str, size_t size, const char *fmt, va_list va)
 {
 	int rc;

[ktmf01]

Thanks!

Perhaps better to detect winapifamily.h in configure.ac and CMakeLists.txt instead of through __has_include, to make this consistent with the rest of the code?

[sezero]

Perhaps better to detect winapifamily.h in configure.ac and CMakeLists.txt instead of through __has_include, to make this consistent with the rest of the code?

That works too I think, since you don't have build systems not relying on config.h such as MSVC project files.

[dmitrykos]

@sezero could you please check what symbol is undefined in your case because we are actually checking if WINAPI_FAMILY_PARTITION is defined that guarantees compatibility if winapifamily.h is not available. Would you please check in your environment what is the value of WINAPI_FAMILY_PARTITION.

Checking for existence of winapifamily.h does not make much sense as we can easily getaway with checking for existence of WINAPI_FAMILY_PARTITION.

It may fail though if your environment has some incorrect stub for WINAPI_FAMILY_PARTITION.

[sezero]

WINAPI_FAMILY_PARTITION is not defined for me. The actual problem is WINAPI_FAMILY_PARTITION() macro not being present.

To visualize the problem better, try to compile this:

#if defined(FOO) && (FOO < BAR(HAR))
#define FUBAR
#endif

[dmitrykos]

Actually we are checking for existence of WINAPI_FAMILY_PARTITION define, so if it is not present the 2nd check for WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_APP) will not be accessed by compiler.

#if defined(WINAPI_FAMILY_PARTITION) &&\
	WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_APP) && !WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)

Please double check once gain maybe? Maybe you have some stub like #define WINAPI_FAMILY_PARTITION somewhere.

[sezero]

As I said, I don't have wihapifamily.h and I don't have any of the related macros defined.

Did you try to build the example I gave you above? it fails the exact same way because BAR() is not defined:
test.c:1:31: error: missing binary operator before token "("

[ktmf01]

Maybe

#if defined(WINAPI_FAMILY_PARTITION) &&\
	WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_APP) && !WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)

Needs to be explicitly split up into

#if defined(WINAPI_FAMILY_PARTITION)
#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_APP) && !WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)

I don't know whether the C preprocessor does short-circuit evaluation?

[dmitrykos]

@sezero you are right. My misassumption was that compiler will not do a 2-nd check if first fails (just like during run-time) but it is not true in case of pre-processor.

So implementation shall be fixed to such:

#if defined(WINAPI_FAMILY_PARTITION)
    #if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_APP) && !WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)
        #define FLAC_WINDOWS_APP 1
    #else
        #define FLAC_WINDOWS_APP 0
    #endif
#else
    #define FLAC_WINDOWS_APP 0
#endif

I will provide a fix for it later in a separate PR.

p.s.: @ktmf01 yes, you are right.

[ktmf01]

So, apparently GCC's preprocessor does short-circuit evaluation, but MSVC does not? Or at least it seems it (older versions) tries to parse the entire expression before evaluating?

https://stackoverflow.com/questions/24037595/is-there-logical-short-circuiting-in-the-c-preprocessor
https://developercommunity.visualstudio.com/t/1468786

Anyway, @dmitrykos thanks, I'll await your PR.

[dmitrykos]

I just created new PR #610 to fix compile error reported by @sezero.

So, apparently GCC's preprocessor does short-circuit evaluation, but MSVC does not? Or at least it seems it (older versions) tries to parse the entire expression before evaluating?

In my tests short-circuit does not happen in case of GCC and MSVC (Microsoft Visual Studio Community 2019 Version 16.11.23).