Fix compiling for x86_64-w64-mingw32

[mywave82]

Compiling with mingw produces MANY instances like this:

psgplay-git/lib/atari/device.c: In function ‘device_run’:
psgplay-git/include/internal/macro.h:42:39: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
  42 |         (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
     |                                       ^
psgplay-git/include/internal/macro.h:45:18: note: in expansion of macro ‘__is_constexpr’
  45 |                 (__is_constexpr(x) && __is_constexpr(y))
     |                  ^~~~~~~~~~~~~~
psgplay-git/include/internal/compare.h:13:39: note: in expansion of macro ‘__no_side_effects’
  13 |                 (__typecheck(x, y) && __no_side_effects(x, y))
     |                                       ^~~~~~~~~~~~~~~~~
psgplay-git/include/internal/compare.h:23:31: note: in expansion of macro ‘__safe_cmp’
  23 |         __builtin_choose_expr(__safe_cmp(x, y), \
     |                               ^~~~~~~~~~
psgplay-git/include/internal/compare.h:32:25: note: in expansion of macro ‘__careful_cmp’
  32 | #define min(x, y)       __careful_cmp(x, y, <)
     |                         ^~~~~~~~~~~~~
psgplay-git/lib/atari/device.c:182:41: note: in expansion of macro ‘min’
 182 |                         machine_slice = min(machine_slice,
     |   

Verified with this small test program

#include <stdio.h>
#include <stdint.h>

#define __is_constexpr(x) \
        (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * (uintptr_t)0l)) : (int *)(uintptr_t)8)))

int foo(void)
{
        return 10;
}

int a = 10;
int bar(void)
{
        return a;
}


int main(int argc, char *argv[])
{
        printf ("0: %d\n",  __is_constexpr(0));
        printf ("1: %d\n",  __is_constexpr(1));
        printf ("1+1: %d\n",  __is_constexpr(1+1));
        printf ("foo(): %d\n",  __is_constexpr(foo()));
        printf ("bar(): %d\n",  __is_constexpr(bar()));

        printf ("sizeof(int) is %d\n", (int)sizeof (int));
        printf ("sizeof(long int) is %d\n", (int)sizeof (long int));
        printf ("sizeof(long lont int) is %d\n", (int)sizeof (long long int));
        return 0;
}

Which produces this output (both without and with the fix)
on x86_64-w64-mingw32:

0: 1
1: 1
1+1: 1
foo(): 0
bar(): 0
sizeof(int) is 4
sizeof(long int) is 4
sizeof(long lont int) is 8

on x86_64_linux:

0: 1
1: 1
1+1: 1
foo(): 0
bar(): 0
sizeof(int) is 4
sizeof(long int) is 8
sizeof(long lont int) is 8

Same issue in lib/toslibc/include/internal/macro.h

[mywave82]

(which brings up another issue - I avoided %zu - which is not available on default mingw due to msvcrt.dll not being C99 and uses %Iu instead)

[frno7]

Hmm... I think it’d be much better to only replace (long) with (intptr_t), and 0l with 0, so simply:

(sizeof(int) == sizeof(*(8 ? ((void *)((intptr_t)(x) * 0)) : (int *)8)))

Rationale:

• The original code had signed integers, and there’s no need to unsign them now, is it?
• The problem, according to the compiler error message, only seems to be the combination (void *)(long), so there’s no need to do anything about (int *)8, is it?
• The l in 0l should be unnecessary due the integer promotion rules mentioned in Remove global variables #66 (reply in thread). The reason that (long)(x) * (uintptr_t)0l is accepted by the compiler is, again, due to the integer promotion by the * operator. Given the operands (long) * (uintptr_t) this implicitly becomes the largest of the two types, so (uintptr_t) * (uintptr_t), hence having a (long) is quite dubious? Not to mention the problem of different signedness as well?

[mywave82]

Yes, it seems to make mingw happy:
(sizeof(int) == sizeof(*(8 ? ((void *)((intptr_t)(x) * 0l)) : (int *)8)))

or just 0 instead of 0l

Online compiler explorer

[frno7]

(which brings up another issue - I avoided %zu - which is not available on default mingw due to msvcrt.dll not being C99 and uses %Iu instead)

Maybe we can define our own PRIuz (similar to PRIu64 etc. from <inttypes.h>), like

#if defined(__MINGW32__)
#define PRIuz "Iu"
#else
#define PRIuz "zu"
#endif

so that, for example,

printf("header size %zu\n", header_size);

becomes

printf("header size %" PRIuz "\n", header_size);

?

[mywave82]

Regarding %zd It is not that simple (tm) if I understand the Internet.

On Windows it depends on which libc variant you link with. And if you are using mingw, you can opt in to override printf() with mingw own:

#define __USE_MINGW_ANSI_STDIO 1

I only have minor experience due to providing windows build (using mingw) of Open Cubic Player, but do no other C programming on Windows.

[frno7]

TOS/libc doesn’t have intptr_t, but I can fix that. :-)

[frno7]

There you go: frno7/toslibc@4f93d3a

[frno7]

I suggest we merge 5d21139, okay?

[mywave82]

My last update has compiletime workaround for systems lacking intptr_t

[frno7]

My last update has compiletime workaround for systems lacking intptr_t

Is there any known (and relevant) system that doesn’t have intptr_t? If not, I suggest we assume that intptr_t always exists (until proven otherwise), as our code becomes less cluttered.

[mywave82]

build-atari-st failed here on github in your CI, but probably before you modified toslibc.

You decide ;) Your pet project

[frno7]

build-atari-st failed here on github in your CI, but probably before you modified toslibc.

Yeah, I added intptr_t for TOS/libc in commit frno7/toslibc@4f93d3a. TOS/libc certainly isn’t ‘complete’ in any sense; I maintain and add things to it on a need-to-use-basis for Atari ST applications (of which PSG play itself is an example). :-)

You decide ;) Your pet project

Indeed, but it’s your commit. Merged in commit 5d21139, thanks!

[mywave82]

%zu and %zd appears to be present if c:\windows\system32\msvcrt.dll is from Visual Studio 2015 or newer (internal version 14), So Windows Vista with updates and beyond.

Windows appear to maintain compatibility with their own %Iu and %Id still which was their alternative instead of implementing C99 prior to to this Visual Studio version.

Use #ifdef _WIN32 and use %Id and %Iu if you want to be ultra-portable to old versions of Windows without updates (or 3rd party software installing "newer" Visual Studio runtime). The warnings mingw gcc throws out should be more informative :)

[frno7]

What’s your preferred level of compatibility with Windows for Open Cubic Player?

My preference for PSG play is to keep the code clean, so any workaround (for Windows or otherwise) needs some justification. My focus is Linux; maintaining some level of compatibility with BSDs and Mac OS is usually easy too. The Atari ST doesn’t use the PSG play library as such, but there are minor cases of shared source files (such as the macro definitions we saw yesterday).

[mywave82]

Windows support was added about 3 years ago, until that my fork/remake was unix systems only. Windows is the black sheep in the source code. Many things must be specially handled in Windows, and there are buggy API's that do not get fixed, since it will break earlier applications. So when easily possible I use what is backwards-compatible, and when that is a major hassle I only support newer versions until I get complaints I can can reconsider.

[mywave82]

Preview from Windows build:

[frno7]

@mywave82, what’s your plan or suggestion on the %zu problem, in case you want to keep this pull request open for a while longer? No hurry with Windows on my part, though. :-)

[mywave82]

These are the most obvious options I think:

Don't do anything, and the messages simply outputs %zu on old windows systems, and systems with old version of Visual C libraries installed.

Add the #ifdef _WIN32 and use the Windows special variant, it still works on newer versions.

Use %d or %u and typecast the result of sizeof().

Last option always works and is the least amount of clutter in the source code.

[frno7]

%zu seems to be used in

psgplay/lib/psgplay/sndh.c

Line 117 in c76403a

diag_error(cursor, "tag %s too large for %zu",

and

psgplay/lib/psgplay/sndh.c

Lines 526 to 527 in c76403a

	diag_error(cursor,
	"unrecognised data in %zu bytes at offset %zu",

Others found by grep are not part of the library, so should be safe. Moreover, diag_error only matters if SNDH tags are iterated with

psgplay/include/psgplay/sndh.h

Lines 198 to 209 in c76403a

	/**
	* sndh_for_each_tag_with_diagnostic - iterate over SNDH tags with diagnostic
	* @data: SNDH data
	* @size: size in bytes of SNDH data
	* @diag: pointer to &struct sndh_diagnostic with warning and error callbacks,
	* or %NULL to ignore
	*
	* Note that some tags such as "TIME" and "!#SN" may have multiple values
	* and be given more than once.
	*/
	#define sndh_for_each_tag_with_diagnostic(data, size, diag) \
	sndh_for_each_tag_with_header_size_and_diagnostic((data), (size), NULL, (diag))

(or sndh_for_each_tag_with_header_size_and_diagnostic directly). But OCP doesn’t seem to use SNDH tag diagnostics? Which is fair, since it’s mostly useful for debugging faulty SNDH files anyway. So, no problem? :-)