#376 out123_open uses strtok_r, which is unavailable on MSVC

[Maarten]

msdn documentation:
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/strtok-strtok-l-wcstok-wcstok-l-mbstok-mbstok-l

GitHub actions failure:
https://github.com/madebr/mpg123/actions/runs/12535426276/job/34957336715#step:8:148

Looks like it is part of posix, and documented as "Extension to the ISO C standard":
https://pubs.opengroup.org/onlinepubs/9799919799/functions/strtok.html

[Maarten]

Perhaps also review all MSVC warnings.

This warning is suspicious:
https://github.com/madebr/mpg123/actions/runs/12535426276/job/34957337064#step:8:153
'char *' differs in levels of indirection from 'int'

[Maarten]

Never mind my comment above, it's a consequence of the unknown strtok_r function.

https://github.com/madebr/mpg123/actions/runs/12535426276/job/34957337064#step:8:155
size_t -> int

[Thomas Orgis]

World, are you kidding me? You cannot win. So we need to check for strtok_r() and fall back if not there? Or rather … strtok_s() for Windows, strtok_r() for others?

Relying on thread-local storage is not quite enough, so it needs to be a safer variant.

[Maarten]

FYI, I updated the GitHub workflows in the master-with-github-ci branch on my fork.

[Thomas Orgis]

OK, I should have fixed the strtok_r usage (now detecting and on MSVC hopefully using strtok_s) with revision 5474 and merged .github changes with 5475.

Maybe you can confirm if things are right now

[Maarten]

Indeed, all MSVC architectures are happy again!
https://github.com/madebr/mpg123/actions

You can remove .github/workflows/CMake-MSVC-noyasm.yml.
.github/workflows/CMake-MSVC.yml tests mpg123 with and without yasm.

https://github.com/madebr/mpg123/commit/c43addb5407f3eaa9c0050ea9c3fa0f3e07af4e2#diff-5decae584beea049114a705b5cf56e54c86c67d12349099f81b9ea98ec9b7427L1

[Ozkan Sezer]

You can remove .github/workflows/CMake-MSVC-noyasm.yml

Yes. And it fails miserably already.

[Ozkan Sezer]

You can remove .github/workflows/CMake-MSVC-noyasm.yml

Yes. And it fails miserably already.

Actually if fails because of that arm32 configuration (named amd64_arm in there) :

LINK : fatal error LNK1104: cannot open file 'kernel32.lib'

This is because of the win32 sdk version we had in SDL, no? Is this
not fixable in mpg123 gh actions somehow?

[Thomas Orgis]

I removed the redundant workflow now. Ozkan: is the combined workflow missing something?

[manx]

is the combined workflow missing something?

As far as I can see, only to old, now removed, noyasm workflow did not successfully build.
See https://github.com/madebr/mpg123/actions/runs/12609590335 vs https://github.com/madebr/mpg123/actions/runs/12609590334 .

This is because Microsoft removed 32bit ARM support in the latest SDK, and only the combined workflow accounts for that by explicitly selecting an older SDK for 32bit ARM. See https://github.com/madebr/mpg123/blob/b7db2283aa045699c53a788575003c4b1128f414/.github/workflows/CMake-MSVC.yml#L76.

[Ozkan Sezer]

Ozkan: is the combined workflow missing something?

I thought 32 bit ARM was missing, but I obviously missed it in the new combined workflow: So, my mistake, everything is OK.

[Maarten]

Microsoft does not support 32-bit ARM in the latest Windows SDK.
I can keep it building by using an older SDK version.
https://github.com/madebr/mpg123/blob/6e06824f9bc16f4fc247a00c331b69c1f8148c4e/.github/workflows/CMake-MSVC.yml#L76

[Maarten]

(not to self: read the chat history before posting)
Apologies about the noise

[Thomas Orgis]

Note: release 1.33.0 should ship with the fix.