Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot build anymore Fluidsynth with MinGW and CYGWIN. #1419

Closed
carlo-bramini opened this issue Nov 5, 2024 · 0 comments · Fixed by #1423
Closed

Cannot build anymore Fluidsynth with MinGW and CYGWIN. #1419

carlo-bramini opened this issue Nov 5, 2024 · 0 comments · Fixed by #1423
Labels

Comments

@carlo-bramini
Copy link
Contributor

carlo-bramini commented Nov 5, 2024

Hello, I downloaded the new version 2.4.0 and I tried to update my packages for MinGW and CYGWIN.
Unfortunately, I got some issues which blocked my builds.

(1) It seems that it has been added C90 as minimum dialect when compiling here:

set(CMAKE_C_STANDARD 90)

but unfortunately there is this piece of code which blocks the process:

fluidsynth-2.4.0/src/utils/fluid_sys.c:1803:5: error: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
 1803 |     static char ascii_err[sizeof(err)];
      |     ^~~~~~

So, in my opinion the fastest way to fix this issue is to declare err[] and ascii_err[] with the same size, by using a macro.

(2) Fluidsynth now adds _UNICODE and UNICODE on the command line without chance to disable it.
Unfortunately, this is bad because it makes builds of Fluidsynth not working anymore when windows-version is set to 0x0400 and CYGWIN fails with this message:

fluidsynth-2.4.0/src/drivers/fluid_waveout.c:431:20: error: implicit declaration of function ‘wcsicmp’; did you mean ‘wcsncmp’? [-Werror=implicit-function-declaration]
  431 |                 if(wcsicmp(lpwDevName, caps.szPname) == 0)
      |                    ^~~~~~~
      |                    wcsncmp

Actually, CYGWIN provides support for widechar but it is different from the one provided by MinGW and MSVC because wchar_t is 32bit as it is on GLibc for linux, for example. it supports wcscasecmp() instead.
In my opinion, it would be better to make an option like this one into CMakeLists.txt:

option ( enable-unicode "enable UNICODE build for Windows" on )

which is set to on by default, so it will work as it works now if it is not provided.
Later, testing enable-unicode will allow to add or not UNICODE for Windows and CYGWIN.

(3) I got a minor warning when building winmidi driver:

fluidsynth-2.4.0/src/drivers/fluid_winmidi.c:492:15: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long long unsigned int’ [-Wsign-compare]
  492 |         if (n >= sizeof(new_name))
      |               ^~

But hopefully, this is easy to fix since it is just needed to declare n as unsigned int instead of int.

I will provide patches for solving these problems if you want.

@derselbst derselbst linked a pull request Nov 6, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant