meson: Start treating warnings as errors

Make it a bit more explicit that we're ignoring some(many) warnings

`-w` ignored *all* warnings. Which is 1) not very sane and 2) easy to miss

`-Werror` will
 1. ensure we don't add new categories of bugs in new code/refactors (some of the changes here are evidence that I, for one, leaned too much on "no compile error? No problem")
 2. allow us to fix those warnings step by step

Some warnings are not attached to a category and thus cannot be disabled individually. Using `-Werror` turned those into errors, which forced me to fix them.

Note: Not tested with clang

Co-authored-by: Rafael Brune <mail@rbrune.de>

Fix windows redefine errors

Fix strict aliasing errors by using copies instead of casts

Linux/flake.nix

@@ -30,6 +30,10 @@
         ];
         # With --enable-sanitizers dlopen() fails to find libs. This is used by SDL2 to open the sound library (pipewire), and by libglvnd to open the vendor-specific OpenGL driver
         LD_LIBRARY_PATH = "${pipewire.lib}/lib;/run/opengl-driver/lib";
+        # The (wrapped) gcc provided by this nix env has source fortifications enabled because its the same gcc that's used to compile nix packages.
+        # However we are in a dev build env and we'd like to have sanitizers enabled. They require `-O0` which is incompatible with source fortification
+        # `error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O)`
+        hardeningDisable = [ "all" ];
       };
 
     source-code = with nixpkgs.legacyPackages.x86_64-linux; runCommand "Source Code"

meson.build

@@ -15,11 +15,56 @@ base_deps = [
 	meson.get_compiler('c').find_library('m', required: false),
 ]
 
-c_base_args = ['-DHW_GAME_HOMEWORLD']
-c_base_args += ['-DHAVE_CONFIG_H', '-D_REENTRANT']
-#c_base_args += '-Wno-unused-variable'
-#c_base_args += '-Wno-unused-parameter'
-c_base_args += '-w'
+c_base_args = [
+	'-DHW_GAME_HOMEWORLD',
+	'-DHAVE_CONFIG_H',
+	'-D_REENTRANT',
+
+	# Treat all warnings as errors. We want all new code to be as safe & clean as possible
+	'-Werror',
+
+	# Ignore existing issues in the codebase
+	# TODO: re-enable them one by one to fix them
+	'-Wno-address', # 2 occurrences
+	'-Wno-cast-function-type',
+	'-Wno-discarded-qualifiers',
+	'-Wno-enum-conversion', # 1 occurrence
+	'-Wno-empty-body',
+	'-Wno-format-security',
+	'-Wno-format-zero-length',
+	'-Wno-format',
+	'-Wno-implicit-fallthrough',
+	'-Wno-implicit-function-declaration',
+	'-Wno-incompatible-pointer-types',
+	'-Wno-int-conversion', # 6 occurrences
+	'-Wno-int-in-bool-context',
+	'-Wno-int-to-pointer-cast',
+	'-Wno-logical-not-parentheses',
+	'-Wno-maybe-uninitialized',
+	'-Wno-missing-field-initializers', # 1 occurrence
+	'-Wno-misleading-indentation', # 2 occurrences
+	'-Wno-multistatement-macros',
+	'-Wno-parentheses',
+	'-Wno-pedantic',
+	'-Wno-pointer-arith',
+	'-Wno-pointer-to-int-cast',
+	'-Wno-restrict',
+	'-Wno-sign-compare',
+	'-Wno-stringop-overflow',
+	'-Wno-stringop-overread',
+	'-Wno-type-limits',
+	'-Wno-unused-but-set-parameter',
+	'-Wno-unused-but-set-variable',
+	'-Wno-unused-function',
+	'-Wno-unused-parameter',
+	'-Wno-unused-value',
+	'-Wno-unused-variable',
+
+	# TODO: Windows mingw build issues
+	'-Wno-array-bounds',
+	'-Wno-deprecated-declarations',
+	'-Wno-stringop-truncation',
+]
 
 if build_machine.cpu_family() == 'x86_64'
 	c_base_args += ['-D_X86_64', '-D_X86_64_FIX_ME', '-DGENERIC_ETGCALLFUNCTION']

src/Game/AIEvents.c

@@ -668,7 +668,7 @@ void aiePreFixAIEvents(struct AITeamMove *move)
         // convert pointer to offset into AIPlayer structure
         dbgAssertOrIgnore(move->events.interrupt.intvar);
         move->events.interrupt.intvar = (udword *)(((ubyte *)move->events.interrupt.intvar) - ((ubyte *)fixingThisAIPlayer));
-        dbgAssertOrIgnore(move->events.interrupt.intvar < sizeof(AIPlayer));
+        dbgAssertOrIgnore(move->events.interrupt.intvar < (udword*)sizeof(AIPlayer));
     }
 
     move->events.gettingRocked.handler  = (aieHandlerShips)aieHandlerToNum((aieHandlerSimple)move->events.gettingRocked.handler);
@@ -706,7 +706,7 @@ void aieFixAIEvents(struct AITeamMove *move)
 
     if (move->events.interrupt.handler)
     {
-        dbgAssertOrIgnore(move->events.interrupt.intvar < sizeof(AIPlayer));
+        dbgAssertOrIgnore(move->events.interrupt.intvar < (udword*)sizeof(AIPlayer));
         move->events.interrupt.intvar = (udword *)( ((ubyte *)fixingThisAIPlayer) + ((sdword)move->events.interrupt.intvar) );
     }
 }

src/Game/BigFile.c

@@ -2717,7 +2717,7 @@ void bigFilesystemCompare(char *baseDirectory, char *directory)
     udword fileNum;
     udword bigfile_i = 0;
     static udword compared = 0;
-    static udword numOverriddenLocal[NUMBER_CONFIGURED_BIG_FILES] = {0,0,0};
+    static udword numOverriddenLocal[NUMBER_CONFIGURED_BIG_FILES] = {0};
 
     if (IgnoreBigfiles || !CompareBigfiles)
     {

src/Game/ETG.c

@@ -411,6 +411,17 @@ void etgDepthWriteResolve(struct etgeffectstatic *stat, etgfunctioncall *call);
 
 
 #ifdef GENERIC_ETGCALLFUNCTION
+#undef funcEntryn
+#undef funcEntry0
+#undef funcEntry1
+#undef funcEntry2
+#undef funcEntry3
+#undef funcEntry4
+#undef funcEntry5
+#undef funcEntry6
+#undef funcEntry7
+#undef funcEntry8
+
 #define funcEntryn(name, ret, function)                                      {name, function, wrap_##function, ret, (ubyte)FALSE, ETG_VariableParams}
 #define funcEntry0(name, ret, function)                                      {name, function, wrap_##function, ret, (ubyte)FALSE, NULL, 0, {0,0,0,0,0,0,0,0,0}}
 #define funcEntry1(name, ret, function, p0)                                  {name, function, wrap_##function, ret, (ubyte)FALSE, NULL, 1, {p0}}
@@ -422,6 +433,17 @@ void etgDepthWriteResolve(struct etgeffectstatic *stat, etgfunctioncall *call);
 #define funcEntry7(name, ret, function, p0, p1, p2, p3, p4, p5, p6)          {name, function, wrap_##function, ret, (ubyte)FALSE, NULL, 7, {p0, p1, p2, p3, p4, p5, p6}}
 #define funcEntry8(name, ret, function, p0, p1, p2, p3, p4, p5, p6, p7)      {name, function, wrap_##function, ret, (ubyte)FALSE, NULL, 7, {p0, p1, p2, p3, p4, p5, p6, p7}}
 
+#undef funcEntryThisn
+#undef funcEntryThis0
+#undef funcEntryThis1
+#undef funcEntryThis2
+#undef funcEntryThis3
+#undef funcEntryThis4
+#undef funcEntryThis5
+#undef funcEntryThis6
+#undef funcEntryThis7
+#undef funcEntryThis8
+
 #define funcEntryThisn(name, ret, function)                                  {name, function, wrap_##function, ret, (ubyte)TRUE,  ETG_VariableParams}
 #define funcEntryThis0(name, ret, function)                                  {name, function, wrap_##function, ret, (ubyte)TRUE,  NULL, 0, {0,0,0,0,0,0,0,0,0}}
 #define funcEntryThis1(name, ret, function, p0)                              {name, function, wrap_##function, ret, (ubyte)TRUE,  NULL, 1, {p0}}
@@ -433,9 +455,15 @@ void etgDepthWriteResolve(struct etgeffectstatic *stat, etgfunctioncall *call);
 #define funcEntryThis7(name, ret, function, p0, p1, p2, p3, p4, p5, p6)      {name, function, wrap_##function, ret, (ubyte)TRUE,  NULL, 7, {p0, p1, p2, p3, p4, p5, p6}}
 #define funcEntryThis8(name, ret, function, p0, p1, p2, p3, p4, p5, p6, p7)  {name, function, wrap_##function, ret, (ubyte)TRUE,  NULL, 7, {p0, p1, p2, p3, p4, p5, p6, p7}}
 
+#undef funcEntryR1
+#undef funcEntryR2
+
 #define funcEntryR1(name, ret, function, p0, resolve)                        {name, function, wrap_##function, ret, (ubyte)FALSE, resolve, 1, {p0}}
 #define funcEntryR2(name, ret, function, p0, p1, resolve)                    {name, function, wrap_##function, ret, (ubyte)FALSE, resolve, 2, {p0, p1}}
 
+#undef funcEntryThisR1
+#undef funcEntryThisR2
+
 #define funcEntryThisR1(name, ret, function, p0, resolve)                    {name, function, wrap_##function, ret, (ubyte)TRUE,  resolve, 1, {p0}}
 #define funcEntryThisR2(name, ret, function, p0, p1, resolve)                {name, function, wrap_##function, ret, (ubyte)TRUE,  resolve, 2, {p0, p1}}
 
@@ -8021,7 +8049,7 @@ udword etgFloat2Int(real32 f)
 udword etgInt2Float(sdword f)
 {
     real32 retVal = (real32)f;
-    return(TreatAsUdword(retVal));
+    return Real32ToUdword(retVal);
 }
 
 udword etgInts2Color(sdword red, sdword green, sdword blue)

src/Game/ETG.h

@@ -465,7 +465,8 @@ typedef struct
 }
 opfunctionentry;
 
-#define END_OP_FUNCTION_ENTRY  {NULL, NULL, 0, 0, 0, 0, {0, 0, 0, 0, 0, 0, 0, 0, 0}}
+#define END_OP_FUNCTION_ENTRY                                                                                          \
+    { 0 }
 
 //entry in a variable-name list for this effect
 typedef struct

src/Game/Mesh.c

@@ -701,19 +701,19 @@ sdword meshLoadAndConvertTo64Bit(char *fileName, void** loadAddress)
         newpolobj[i].nPolygons = oldpolobj[i].nPolygons;
         newpolobj[i].localMatrix = oldpolobj[i].localMatrix;
 
-        if (oldpolobj[i].pDaughter != NULL){
+        if (oldpolobj[i].pDaughter != 0) {
             newpolobj[i].pDaughter = ( oldpolobj[i].pDaughter - sizeof(GeoFileHeader_disk)) /
                  sizeof(struct polygonobject_disk) * sizeof(struct polygonobject) + sizeof(GeoFileHeader);
         }
-        if (oldpolobj[i].pMother != NULL){
+        if (oldpolobj[i].pMother != 0) {
             newpolobj[i].pMother = ( oldpolobj[i].pMother - sizeof(GeoFileHeader_disk)) /
                  sizeof(struct polygonobject_disk) * sizeof(struct polygonobject) + sizeof(GeoFileHeader);
         }
-        if (oldpolobj[i].pSister != NULL){
+        if (oldpolobj[i].pSister != 0) {
             newpolobj[i].pSister = ( oldpolobj[i].pSister - sizeof(GeoFileHeader_disk)) /
                  sizeof(struct polygonobject_disk) * sizeof(struct polygonobject) + sizeof(GeoFileHeader);
         }
-    
+
         newptr += sizeof(struct polygonobject);
         offset += sizeof(struct polygonobject);
 
@@ -788,13 +788,13 @@ sdword meshLoadAndConvertTo64Bit(char *fileName, void** loadAddress)
     }
         
     for (i=0; i < numPolygons; i++) {
-        if (oldpolobj[i].pName != NULL) {
-         strcpy(newptr, oldbase + oldpolobj[i].pName);
-         loopsize=strlen(oldbase + oldpolobj[i].pName) + 1;
-         newpolobj[i].pName = offset;
+        if (oldpolobj[i].pName != 0) {
+            strcpy(newptr, oldbase + oldpolobj[i].pName);
+            loopsize = strlen(oldbase + oldpolobj[i].pName) + 1;
+            newpolobj[i].pName = offset;
 
-         newptr += loopsize;
-         offset += loopsize;  
+            newptr += loopsize;
+            offset += loopsize;
         }
     }
 
@@ -1247,8 +1247,8 @@ void meshRecolorize(meshdata *mesh)
     for (index = 0; index < mesh->nLocalMaterials + mesh->nPublicMaterials; index++)
     {                                                       //for all materials in mesh
         //3 states of this member:TR_Invalid, pointer to list of texture handles or 0 (no texture)
-        if (mesh->localMaterial[index].texture != TR_Invalid && mesh->localMaterial[index].texture != 0)
-        {                                                   //if textured material
+        if (mesh->localMaterial[index].texture != (trhandle*)TR_Invalid &&
+            mesh->localMaterial[index].texture != 0) { // if textured material
             if (!mesh->localMaterial[index].bTexturesRegistered)
             {                                               //if textures were never registered properly
                 meshTextureNameToPath(fullName, mesh->fileName, (char *)mesh->localMaterial[index].texture);

src/Game/ShipView.c

@@ -36,7 +36,6 @@
 #include "Universe.h"
 
 #ifdef _WIN32
-    #define WIN32_LEAN_AND_MEAN
     #include <windows.h>
 #endif
 

src/Game/TradeMgr.c

@@ -51,7 +51,6 @@
 #include "Vector.h"
 
 #ifdef _WIN32
-    #define WIN32_LEAN_AND_MEAN
     #include <windows.h>
 #endif
 

src/Game/Types.c

@@ -20,25 +20,35 @@ float SwapFloat32( float val )
 
 udword Real32ToUdword(real32 a)
 {
- return (*(udword*)(&a));
+    udword tmp;
+    memcpy(&tmp, &a, sizeof(udword));
+    return tmp;
 }
 
 udword Real32ToSdword(real32 a)
 {
- return (*(sdword*)(&a));
+    udword tmp;
+    memcpy(&tmp, &a, sizeof(udword));
+    return tmp;
 }
 
 real32 UdwordToReal32(udword a)
 {
- return (*(real32*)(&a));
+    real32 tmp;
+    memcpy(&tmp, &a, sizeof(real32));
+    return tmp;
 }
 
 real32 SdwordToReal32(sdword a)
 {
- return (*(real32*)(&a));
+    real32 tmp;
+    memcpy(&tmp, &a, sizeof(real32));
+    return tmp;
 }
 
 real32 MemsizeToReal32(memsize a)
 {
-    return (*(real32 *)(&a));
+    real32 tmp;
+    memcpy(&tmp, &a, sizeof(real32));
+    return tmp;
 }

src/Game/Types.h

@@ -259,10 +259,13 @@ real32 MemsizeToReal32(memsize a);
         (b) ^= (a);     \
         (a) ^= (b);
 //floating-point swap
-#define swapReal32(a, b)                              \
-        *((udword *)(&(a))) ^= *((udword *)(&(b)));   \
-        *((udword *)(&(b))) ^= *((udword *)(&(a)));   \
-        *((udword *)(&(a))) ^= *((udword *)(&(b)));
+#define swapReal32(a, b) swapReal32_internal(&(a), &(b))
+static inline void swapReal32_internal(real32 *a, real32 *b)
+{
+    real32 temp = *a;
+    *a = *b;
+    *b = temp;
+}
 
 #define strdollar(x) #x
 

src/Game/wrap.h

@@ -1,3 +1,5 @@
+#pragma once
+
 #define get_EVT_Constant(n)      do_get_arg(effect, stat, opcode, n)
 #define get_EVT_ConstLabel(n)    do_get_arg(effect, stat, opcode, n)
 #define get_EVT_VarLabel(n)      do_get_arg(effect, stat, opcode, n)

src/SDL/font.c

@@ -681,9 +681,9 @@ sdword fontLoadAndConvertTo64Bit(char *fileName, void** loadAddress)
 
     oldlength = fileLoadAlloc(fileName, (void **)&oldrootptr, NonVolatile);
 
-    oldfontfileheader = oldptr = oldrootptr;  
+    oldfontfileheader = oldptr = oldrootptr;
 
-    newfontfileheader = newptr = newrootptr = newrootptr = memAlloc(2 * oldlength, "font 64bit conversion", 0);
+    newfontfileheader = newptr = newrootptr = memAlloc(2 * oldlength, "font 64bit conversion", 0);
     memset(newrootptr, 0,2*oldlength);
 
     strncpy(newfontfileheader->identification, oldfontfileheader->identification, 8);

src/SDL/mainrgn.c

@@ -79,7 +79,6 @@
 #include "utility.h"
 
 #ifdef _WIN32
-    #define WIN32_LEAN_AND_MEAN
     #include <windows.h>
 #endif
 

src/SDL/mouse.c

@@ -42,7 +42,6 @@
 #include "utility.h"
 
 #ifdef _WIN32
-    #define WIN32_LEAN_AND_MEAN
     #include <windows.h>
 #endif
 

src/SDL/prim2d.c

@@ -19,7 +19,6 @@
 #include "rglu.h"
 
 #ifdef _WIN32
-    #define WIN32_LEAN_AND_MEAN
     #include <windows.h>
 #endif
 

src/SDL/prim3d.c

@@ -20,7 +20,6 @@
 #include "render.h"
 
 #ifdef _WIN32
-    #define WIN32_LEAN_AND_MEAN
     #include <windows.h>
 #endif
 

src/SDL/render.c

@@ -2333,7 +2333,7 @@ void rndMainViewAllButRenderFunction(Camera *camera)
     ;
 }
 
-static enum WhetherToDraw {
+enum WhetherToDraw {
     Draw = 1,
     DontDraw = 0,
 };
@@ -2344,7 +2344,7 @@ static enum WhetherToDraw onRenderEffect(Effect* effect) {
     if (effectowner->objtype != OBJ_BulletType) return Draw;
 
     // effect is owned by a bullet
-    Bullet const* const bullet = (Bullet const*)effectowner;
+    Bullet* const bullet = (Bullet const*)effectowner;
     if (bullet->bulletType == BULLET_Beam && bullet->timelived <= UNIVERSE_UPDATE_PERIOD) {
         return DontDraw;
     }

src/SDL/soundlow.h

@@ -19,10 +19,6 @@
 #define SND_LOOP            0x0008  /* loop the sound until next sndPlaySound */
 #define SND_NOSTOP          0x0010  /* don't stop any currently playing sound */
 
-#define SND_NOWAIT	0x00002000L /* don't wait if the driver is busy */
-#define SND_ALIAS       0x00010000L /* name is a registry alias */
-#define SND_ALIAS_ID	0x00110000L /* alias is a predefined ID */
-
 #define SOUND_MAX_VOICES	32		// Maximum number of voices
 #define SOUND_DEF_VOICES	16		// Default number of voices
 #define SOUND_MIN_VOICES	8		// Minimum number of voices

src/SDL/sstream.c

@@ -1369,13 +1369,9 @@ Recover:
 				streamer.status = SOUND_PLAYING;
 			}
 		}
-		
-#ifdef _MSC_VER
-#pragma message("This should use semaphores!")
-#else
-#warning This should use semaphores!
-#endif
-		SDL_Delay(SOUND_STREAM_SLEEP);
+
+                // TODO: Use semaphores !
+                SDL_Delay(SOUND_STREAM_SLEEP);
 	}
 	
 	/* clean up all the streams */

src/Ships/DFGFrigate.c

@@ -124,7 +124,7 @@ void univDFGFieldEffect(Ship *ship, Bullet *bullet,real32 totaltimeelapsed)
 //            matCreateCoordSysFromHeading(&newCoordsys,&shiptobullet);
             floatDamage = (real32)bullet->damage;
             floatDamage *= ship->magnitudeSquared;
-            intDamage = TreatAsUdword(floatDamage);
+            intDamage = Real32ToUdword(floatDamage);
             etgEffectCreate(stat, ship, &bullet->posinfo.position, NULL, &tmpmatrix, ship->magnitudeSquared, EAF_Velocity | EAF_NLips, 1, intDamage);
         }
     }

src/Ships/DefenseFighter.c

@@ -170,9 +170,9 @@ void defensefightertargetbullet(Ship *ship, Bullet *bullettotarget)
 
     //Laser effect...
     floatDamage = (real32)laser->damage;
-    intDamage = TreatAsUdword(floatDamage);
-    intVelocity = TreatAsUdword(gunstatic->bulletspeed);
-    intLength = TreatAsUdword(gunstatic->bulletlength);
+    intDamage = Real32ToUdword(floatDamage);
+    intVelocity = Real32ToUdword(gunstatic->bulletspeed);
+    intLength = Real32ToUdword(gunstatic->bulletlength);
     //create an effect for bullet, if applicable
 
     etgLOD = etgGunEventTable[shipstatic->shiprace][gunstatic->gunsoundtype][EGT_GunBullet];//get pointer to bullet effect

src/Ships/GravWellGenerator.c

@@ -117,13 +117,13 @@ void runShipEffect(Ship *ship)
          {
              udword colSizeDword;
              dbgAssertOrIgnore(ship->objtype == OBJ_ShipType);
-             colSizeDword = TreatAsUdword(((ShipStaticInfo *)(ship->staticinfo))->staticheader.staticCollInfo.collspheresize);
+             colSizeDword = Real32ToUdword(((ShipStaticInfo *)(ship->staticinfo))->staticheader.staticCollInfo.collspheresize);
              etgEffectCreate(stat, ship, &ship->posinfo.position, &ship->posinfo.velocity, &ship->rotinfo.coordsys, ship->magnitudeSquared, EAF_Full, 1, colSizeDword);
 			 {
 				real32 realtemp;
 				udword temp;
 				temp = etgFRandom(gravwellEffectLifeLow,gravwellEffectLifeHigh);
-				realtemp = *((real32 *)(&temp));
+				realtemp = UdwordToReal32(temp);
 				ship->gravwellTimeEffect = universe.totaltimeelapsed + realtemp;
 			 }
          }

src/Ships/RepairCorvette.c

@@ -174,8 +174,8 @@ void startRepairEffect(Ship *ship,SpaceObjRotImpTarg *target,vector *trajectory,
 
     matCreateCoordSysFromHeading(&coordsys,trajectory);
 
-    intLength = TreatAsUdword(distance);
-    intWidth = TreatAsUdword(targetRadius);
+    intLength = Real32ToUdword(distance);
+    intWidth = Real32ToUdword(targetRadius);
 
     vecAddTo(repairBeamPosition,ship->posinfo.position);