Opened 5 years ago

Closed 5 years ago

#11278 closed defect (fixed)

BUILD: Build failure on unstable Debian (FluidSynth 2.1.0)

Reported by: eriktorbjorn Owned by: digitall
Priority: normal Component: Audio
Version: Keywords:
Cc: Game:

Description

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?

Change History (6)

comment:1 by digitall, 5 years ago

Owner: set to digitall
Resolution: fixed
Status: newpending

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.

comment:2 by eriktorbjorn, 5 years ago

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?

comment:3 by eriktorbjorn, 5 years ago

Resolution: fixed
Status: pendingnew

comment:4 by digitall, 5 years ago

Component: --Other--Audio
Resolution: fixed
Status: newpending

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 :/

comment:5 by eriktorbjorn, 5 years ago

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

comment:6 by digitall, 5 years ago

Status: pendingclosed

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.

Note: See TracTickets for help on using tickets.