Closed kwyxz closed 6 years ago
hmmm problem here is we're keeping this upstream friendly so maybe wait for a review or lets ask upstream for a solution.
I think it should be no big deal to push through a variable name change if it can enable a platform to work like Solaris.
@barbudreadmon Is it possible this change can be pushed through to upstream?
Adding "-USS" (= undefine SS system macro) to the gcc cflags for solaris won't work ?
Edit : i commited a potential fix, let me know if it works. If not, i'll ask upstream about this.
Hi @barbudreadmon ! First thing I tried was to #undef SS in the header file but it did not work as expected, and I'm afraid using -USS has the same results:
g++ -c -osrc/cpu/nec/nec.o src/cpu/nec/nec.cpp -std=gnu++98 -DGIT_VERSION=\"" 8ed5e87"\" -O3 -DNDEBUG -fPIC -DPTR64 -DINCLUDE_7Z_SUPPORT -D__LIBRETRO__ -DLSB_FIRST -USS -D__LIBRETRO_OPTIMIZATIONS__ -DUSE_SPEEDHACKS -DTOAPLAN_SOUND_SAMPLES_HACK -fforce-addr -finline-limit=1200 -fcheck-new -Wall -W -Wshadow -Wno-long-long -Wno-write-strings -Wunknown-pragmas -Wundef -Wno-conversion -Wno-missing-braces -Wno-multichar -Wuninitialized -Wpointer-arith -Wno-inline -Wno-unused-value -Wno-sequence-point -Wno-extra -Wno-strict-aliasing -Wno-write-strings -pedantic -Wno-address -Wno-unused-but-set-variable -Wno-narrowing -Wno-pedantic -DFRONTEND_SUPPORTS_RGB565 -Isrc/burner/win32 -Isrc/burner/libretro/libretro-common/include -Isrc/burner/libretro -Isrc/burn -Isrc/cpu -Isrc/burn/snd -Isrc/burn/devices -Isrc/intf -Isrc/intf/input -Isrc/intf/cd -Isrc/intf/audio -Isrc/burner -Isrc/cpu -Isrc/cpu/i8039 -Isrc/cpu/i8051 -Isrc/cpu/tms32010 -Isrc/cpu/upd7725 -Isrc/cpu/upd7810 -Isrc/cpu/v60 -Isrc/dep/libs/zlib -Isrc/dep/libs/lib7z -Isrc/burn/drv/capcom -Isrc/burn/drv/konami -Isrc/burn/drv/dataeast -Isrc/burn/drv/cave -Isrc/burn/drv/neogeo -Isrc/burn/drv/psikyo -Isrc/burn/drv/sega -Isrc/burn/drv/toaplan -Isrc/burn/drv/taito -Isrc/dep/generated -Isrc/dep/libs
In file included from /usr/include/sys/ucontext.h:14:0,
from /usr/include/sys/signal.h:212,
from /usr/include/sys/procset.h:17,
from /usr/include/sys/wait.h:19,
from /usr/include/stdlib.h:15,
from src/cpu/nec/nec.cpp:106:
src/cpu/nec/necpriv.h:77:25: error: expected identifier before numeric constant
typedef enum { DS1, PS, SS, DS0 } SREGS;
^
src/cpu/nec/necpriv.h:77:25: error: expected ‘}’ before numeric constant
src/cpu/nec/necpriv.h:77:25: error: expected unqualified-id before numeric constant
In file included from src/cpu/nec/nec.cpp:141:0:
src/cpu/nec/necpriv.h:77:33: error: expected declaration before ‘}’ token
typedef enum { DS1, PS, SS, DS0 } SREGS;
^
src/cpu/nec/nec.cpp:123:24: warning: ‘void write_mem_word(unsigned int, short unsigned int)’ defined but not used [-Wunused-function]
static NEC_INLINE void write_mem_word(unsigned int a, unsigned short d)
^
src/cpu/nec/nec.cpp:129:24: warning: ‘void write_port_word(unsigned int, short unsigned int)’ defined but not used [-Wunused-function]
static NEC_INLINE void write_port_word(unsigned int a, unsigned short d)
^
gmake: *** [src/cpu/nec/nec.o] Error 1
That's why I went with renaming the variable even though it doesn't seem like the most clever solution :(
I think it's generally good coding practice to avoid using any reserved keyword names for variables, no matter what OS/environment it's from.
@twinaphex i agree, i'll let upstream know about this.
Upstream won't accept those changes, and i have to say i understand their point : solaris is not for gaming, fba is not targeted at solaris, there is no end to fixing specific platform issues (see https://github.com/libretro/fbalpha/issues/178), the new code is less elegant than the old one.
I pushed a commit for this, i also changed the make rule (using platform parameter like the other platforms), i guess i'll have to maintain those changes on the libretro port.
Seems like SS is reserved on Solaris, breaking the build. Renaming it to SS_ fixes the issue, even though it may not be the most elegant solution.