BUILD: Build failure on unstable Debian (FluidSynth 2.1.0)

[eriktorbjorn]

Debian unstable recently updated from FluidSynth 1.1.14 to 2.1.0. Unfortunately, I now get this when I try to compile ScummVM:

    C++      audio/softsynth/fluidsynth.o
In file included from ./common/scummsys.h:472,
                 from audio/softsynth/fluidsynth.cpp:23:
./common/forbidden.h:57:89: error: expected ‘)’ before ‘SYMBOL’
   57 | YMBOL_REPLACEMENT FORBIDDEN_look_at_common_forbidden_h_for_more_info SYMBOL !%*
      |                                                                      ^~~~~~

In file included from /usr/include/fluidsynth.h:105,
                 from audio/softsynth/fluidsynth.cpp:38:
/usr/include/fluidsynth/log.h:82:24: note: to match this ‘(’
   82 | __attribute__ ((format (printf, 2, 3)))
      |                        ^
make: *** [Makefile.common:122: audio/softsynth/fluidsynth.o] Error 1

It can be worked around by adding "#define FORBIDDEN_SYMBOL_EXCEPTION_printf" but that feels a bit kludgy to me. Is there no better way?

[digitall]

Should be fixed by ​https://github.com/scummvm/scummvm/commit/8593a9e1e4e8dd1f5dfac9b1304a417df9a742e6

This at least limits the exception to just that header inclusion.

We could look at modifying the forbidden from say "printf" to "printf(", but even that could be tricky.

I think this fix should be fine for the current issue and is documented in the comment.

eriktorbjorn: Will mark this bug as pending fixed until you confirm my commit fixed the issue for you.

[eriktorbjorn]

For some reason I didn't see any email notification about this, but no matter. I wouldn't have seen it until now anyway.

Unfortunately it doesn't work. The exception apparently has to be made before forbidden.h (i.e. scummsys.h) is included, because that's where printf is redefined.

I have verified that putting the #define before the #includes works for me. I guess there's no point in undefining it afterwards.

Another way to fix it for me would be to have this before including scummsys.h:

#include "config.h"

#ifdef USE_FLUIDSYNTH
#include <fluidsynth.h>
#endif

I need to include config.h because otherwise I don't have USE_FLUIDSYNTH. Usually we don't include config.h directly, but it's not completely unheard of so maybe that's the better solution here?

[digitall]

Ah mea culpa. I see. Have thought about this and have applied a very similar change to your suggestion with config.h and reordering of header inclusion here. Probably the best and least invasive solution while keeping the forbidden functionality for the rest of the source file. This is committed as ​https://github.com/scummvm/scummvm/commit/68758a879e0c8ecc0d40962516d4e808aa4e15e5>>

eriktorbjorn: Will mark this bug as pending fixed until you confirm my commit fixed the issue for you... again.. Sorry about that :/

[eriktorbjorn]

Still not getting email notifications about this for some reason... but now I can compile without errors, so I guess it's fixed. Thanks!

[digitall]

No problem. Will close bug as fixed. Not sure about the e-mail notifications from Trac. Will poke mataniko / rootfather and see what can be done to fix that.