0003905: zandronumbinaries.cpp OS-dependent macro hell is quite ugly (and broken)

#ifdef Q_OS_WIN32 #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "windows.zip" #define ZANDRONUM_BINARY_NAME "zandronum.exe" #define SCRIPT_FILE_EXTENSION ".bat" #else #ifndef __x86_64__ #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "linux-x86.tar.bz2" #else #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "linux-x86_64.tar.bz2" #endif #ifndef Q_OS_DARWIN #define ZANDRONUM_BINARY_NAME "zandronum" #else #define ZANDRONUM_BINARY_NAME "Zandronum.app" #define ZANDRONUM_APP_BUNDLE_BIN "/Contents/MacOS/zandronum" #endif #define SCRIPT_FILE_EXTENSION ".sh" #endif

Questions or other issues? Contact Us.

Links

Doom and Doom II are the property of id Software.

View Revisions: Issue #3905		[ Back to Issue ]
Summary	0003905: zandronumbinaries.cpp OS-dependent macro hell is quite ugly (and broken)

Revision	2021-09-24 15:40 by WubTheCaptain
Description	This macro hell was too cumbersome to read for me: Quote from src/plugins/zandronum/zandronumbinaries.cpp #ifdef Q_OS_WIN32 #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "windows.zip" #define ZANDRONUM_BINARY_NAME "zandronum.exe" #define SCRIPT_FILE_EXTENSION ".bat" #else #ifndef __x86_64__ #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "linux-x86.tar.bz2" #else #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "linux-x86_64.tar.bz2" #endif #ifndef Q_OS_DARWIN #define ZANDRONUM_BINARY_NAME "zandronum" #else #define ZANDRONUM_BINARY_NAME "Zandronum.app" #define ZANDRONUM_APP_BUNDLE_BIN "/Contents/MacOS/zandronum" #endif #define SCRIPT_FILE_EXTENSION ".sh" #endif The first Q_OS_WIN32 seems fine. Then the latter confused me. I forgot it's not possible to re-define a macro without undefining it first in C++. This initially led me to believe 64-bit Windows would download GNU/Linux binaries and use a binary named zandronum, instead of zandronum.exe. After copying the code to my text editor and adding spaces, I figured the first Q_OS_WIN32 block doesn't even end in #endif and changed my understanding of the code entirely. With spaces and newlines, the above code is the same as: #ifdef Q_OS_WIN32 #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "windows.zip" #define ZANDRONUM_BINARY_NAME "zandronum.exe" #define SCRIPT_FILE_EXTENSION ".bat" #else #ifndef __x86_64__ #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "linux-x86.tar.bz2" #else #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "linux-x86_64.tar.bz2" #endif #ifndef Q_OS_DARWIN #define ZANDRONUM_BINARY_NAME "zandronum" #else #define ZANDRONUM_BINARY_NAME "Zandronum.app" #define ZANDRONUM_APP_BUNDLE_BIN "/Contents/MacOS/zandronum" #endif #define SCRIPT_FILE_EXTENSION ".sh" #endif Suddenly more understandable (thanks, spaces!), yet still ugly (and broken in some ways). Other issues here, some which may be seperate issues: __x86_64__ probably isn't portable. Qt handily has Q_PROCESSOR_X86_64 (at compile-time), which I found to be slightly more portable (and not mostly specific to GCC). Anything non-Windows (except Q_OS_DARWIN, which has a special case later) will still download GNU/Linux binaries (because of the current state of the Zandronum downloads web directory). Hello OpenBSD. Zandronum 3.1 (210919-1948) switched its downloadable 64-bit binary distribution filenames to linux-x64.tgz. Q_OS_DARWIN includes iOS and stuff, so perhaps Q_OS_MACOS would be more appropriate (based on the comment). I understand the current state allows the user to configure the exact file for Zandronum stable binaries, yet testing binaries allow the user to configure the testing directory (where we've hard-coded Zandronum testing binary filenames for scripts to be created). I would sort of like this split abstraction to cease existing for code maintainability to reduce the amount of preprocessor macros and inflexibility, although it would likely mean losing support for multiple testing versions of Zandronum without user configuration. (I have thought of alternative solutions to the problem: symlinks or a "firefox.real" approach taken by Debian.)

Revision	2021-09-24 18:06 by WubTheCaptain
Description	This macro hell was too cumbersome to read for me: Quote from src/plugins/zandronum/zandronumbinaries.cpp #ifdef Q_OS_WIN32 #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "windows.zip" #define ZANDRONUM_BINARY_NAME "zandronum.exe" #define SCRIPT_FILE_EXTENSION ".bat" #else #ifndef __x86_64__ #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "linux-x86.tar.bz2" #else #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "linux-x86_64.tar.bz2" #endif #ifndef Q_OS_DARWIN #define ZANDRONUM_BINARY_NAME "zandronum" #else #define ZANDRONUM_BINARY_NAME "Zandronum.app" #define ZANDRONUM_APP_BUNDLE_BIN "/Contents/MacOS/zandronum" #endif #define SCRIPT_FILE_EXTENSION ".sh" #endif The first Q_OS_WIN32 seems fine. Then the latter confused me. I forgot it's not possible to re-define a macro without undefining it first in C++. This initially led me to believe 64-bit Windows would download GNU/Linux binaries, and use a binary named zandronum instead of zandronum.exe. After copying the code to my text editor and adding spaces, I figured the first Q_OS_WIN32 block doesn't even end in #endif and changed my understanding of the code entirely. With spaces and newlines, the above code is the same as: #ifdef Q_OS_WIN32 #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "windows.zip" #define ZANDRONUM_BINARY_NAME "zandronum.exe" #define SCRIPT_FILE_EXTENSION ".bat" #else #ifndef __x86_64__ #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "linux-x86.tar.bz2" #else #define TESTING_BINARY_URL TESTING_BINARY_URL_BASE "linux-x86_64.tar.bz2" #endif #ifndef Q_OS_DARWIN #define ZANDRONUM_BINARY_NAME "zandronum" #else #define ZANDRONUM_BINARY_NAME "Zandronum.app" #define ZANDRONUM_APP_BUNDLE_BIN "/Contents/MacOS/zandronum" #endif #define SCRIPT_FILE_EXTENSION ".sh" #endif Suddenly more understandable (thanks, spaces!), yet still ugly (and broken in some ways). Other issues here, some which may be seperate issues: __x86_64__ probably isn't portable. Qt handily has Q_PROCESSOR_X86_64 (at compile-time), which I found to be slightly more portable (and not mostly specific to GCC). Anything non-Windows (except Q_OS_DARWIN, which has a special case later) will still download GNU/Linux binaries (because of the current state of the Zandronum downloads web directory). Hello OpenBSD. Zandronum 3.1 (210919-1948) switched its downloadable 64-bit binary distribution filenames to linux-x64.tgz. Q_OS_DARWIN includes iOS and stuff, so perhaps Q_OS_MACOS would be more appropriate (based on the comment). I understand the current state allows the user to configure the exact file for Zandronum stable binaries, yet testing binaries allow the user to configure the testing directory (where we've hard-coded Zandronum testing binary filenames for scripts to be created). I would sort of like this split abstraction to cease existing for code maintainability to reduce the amount of preprocessor macros and inflexibility, although it would likely mean losing support for multiple testing versions of Zandronum without user configuration. (I have thought of alternative solutions to the problem: symlinks or a "firefox.real" approach taken by Debian.)