SDL-Hercules-390 / hyperion

The SDL Hercules 4.x Hyperion version of the System/370, ESA/390, and z/Architecture Emulator
Other
237 stars 89 forks source link

Latest changes for Windows break build on Linux #641

Closed mcisho closed 5 months ago

mcisho commented 5 months ago

Attempting to update Hercules to the latest commit and get the following:-

  CC       cckdcdsk.o
In file included from cckdcdsk.c:14:
In file included from ./hercules.h:80:
./w32util.h:277:41: error: unknown type name 'OSVERSIONINFOEX'
  277 | W32_DLL_IMPORT void w32_GetWinVersInfo( OSVERSIONINFOEX* pOSVersInfoEx );
      |                                         ^
1 error generated.
make[2]: *** [Makefile:2616: cckdcdsk.o] Error 1
make[2]: Leaving directory '/home/:::/hyperion'
make[1]: *** [Makefile:2680: all-recursive] Error 1
make[1]: Leaving directory '/home/:::/hyperion'
make: *** [Makefile:2001: all] Error 2

My host system is Fedora 39.

Ian

atncsj6h commented 5 months ago

same for apple ...

Making all in .
  CC       cckdcdsk.o
In file included from ../cckdcdsk.c:14:
In file included from ../hercules.h:80:
../w32util.h:277:41: error: unknown type name 'OSVERSIONINFOEX'
W32_DLL_IMPORT void w32_GetWinVersInfo( OSVERSIONINFOEX* pOSVersInfoEx );
                                        ^
1 error generated.
gmake[2]: *** [Makefile:2614: cckdcdsk.o] Error 1
gmake[1]: *** [Makefile:2678: all-recursive] Error 1
gmake: *** [Makefile:1999: all] Error 2

in hercules.h the

include "w32util.h" / win32 porting functions /

at line 80 is unconditional

( to test I just commented it and everything works on apple sonoma )

also the coding is pretty sloppy ( pretty is the PC forform of f***ing ) in hercules.h there is the include of "w32util.h" in w32util.h thre is the include of hercules.h the same for too many other headers

Fish-Git commented 5 months ago

Sorry about that. w32util.h has apparently been broken for many years. (Since 2005 from the looks of it, believe it or not!) The #endif for _MSVC_ is in the wrong place. It should be at the very end, just before the #endif for _W32UTIL_H. It hasn't caused any problems until now since the DisableInvalidParameterHandling and EnableInvalidParameterHandling function declarations are void functions that don't take any parameters, so non-Windows builds didn't care. My having added the new w32_GetWinVersInfo function declaration however, finally tripped the bug.

It's now been fixed by commit ff6cc5d1f729d2ea5d6ce29927520f5302b0950a. Please update your Herc (git pull) and retry your build. It should work now. Tested on Kubuntu 21.10 and CentOS 6.10.

Closing.

Fish-Git commented 5 months ago

also the coding is pretty sloppy ( pretty is the PC forform of f***ing )

"pretty is the PC forform of f***ing"?  :)

I've never heard that before!

What's it supposed to mean?  

in hercules.h there is the include of "w32util.h" in w32util.h thre is the include of hercules.h the same for too many other headers

Yes, technically the #include for w32util.h in the hercules.h header should be guarded with a #ifdef _MSVC_, but as long as the w32util.h header itself is guarded, it shouldn't make any difference. But #including it in hercules.h without any guard (and having the guard in w32util.h itself instead) is simpler/cleaner IMO.

As far as some headers re-including headers that were already included by some previously included header is concerned, my only response is: "So what?". Any header and/or source module should be free to include whatever other headers it might need regardless of whether or not they've already been #included somewhere else in the compilation unit, since ALL headers should be self-guarded against being included multiple times. So if a header is included somewhere when it was already included elsewhere, no harm should result. That's pretty much standard software programming IMHO.

wrljet commented 5 months ago

also the coding is pretty sloppy ( pretty is the PC forform of f***ing )

"pretty is the PC forform of f***ing"?  :)

I've never heard that before!

What's it supposed to mean?

I think that means "pretty sloppy" is being used to be politically correct and polite and not type "fucking sloppy" in a GitHub comment, like we might do.  :)

Fish-Git commented 5 months ago

I think that means "pretty sloppy" is being used to be politically correct and polite and not type "fucking sloppy" in a GitHub comment, like we might do.  :)

(Doh!) Thanks Bill. I completely missed that.