Trying to fix -Wcast-function-type warnings in rt_menu.c

[malespiaut]

Fabian,

I'm trying to fix all -Wextra warnings, but one of them caught my attention.

rt_menu.c:381:47: warning: cast between incompatible function types from ‘int (*)(int,  int)’ to ‘void (*)(int)’ [-Wcast-function-type]
  381 |       { CP_Active,         "mm_opt2\0",  'R', (menuptr)CP_LoadGame },
      |                                               ^
rt_menu.c:382:47: warning: cast between incompatible function types from ‘int (*)(void)’ to ‘void (*)(int)’ [-Wcast-function-type]
  382 |       { CP_Inactive,       "mm_opt3\0",  'S', (menuptr)CP_SaveGame },
      |

This refers to the fast that menuptr is defined as typedef void (*menuptr)(int);, while the function of menu items are declared as follow:

void CP_NewGame(void);
void CP_BattleModes(void);
int CP_LoadGame(int quick, int dieload);
int CP_SaveGame(void);
void CP_ControlMenu(void);
void CP_OrderInfo(void);
void CP_ViewScores(void);
void CP_Quit(int which);

My first intent was to change all functions to int (*)(int, int), marking and commenting the arguments and return values of previous void (*)(void) explicitely unused.

In other words, void CP_NewGame(void) would become int CP_NewGame(int unused1, unused2), and menuptr would become typedef int (*menuptr)(int, int);:

int CP_NewGame(int unused1, int unused2)
{
  //XXX: Arguments are only used to avoid casting warnings.
  (void)unused1;
  (void)unused2;

  ...

  //XXX: Return value is only used to avoid casting warnings.
  return 0;
}

Not the most elegant solution, but it works, and is difficult to misunderstand.

What's your opinion?

[fabiangreffrath]

We could define a union of different function prototypes and declare each menu function as one of this type.

For example, we are doing something very similar in Doom:

https://github.com/fabiangreffrath/woof/blob/b51cd3096f199d3d957a42941cb7817d948dd1e7/src/d_think.h#L25-L34

[malespiaut]

Interesting. After glancing at the code you have linked, I'm not sure yet to understand how it works, or how it should be used, but I'll dig that in the coming days. That'll be one of the very few cases where unions are useful.

By the way, Woof is a great source port! Definitely one of the few that I prefer to use, along with Chocolate Doom and Crispy Doom! I don't really enjoy the other modern overcomplicated ports, aside from their robust network support.

[fabiangreffrath]

@malespiaut

diff --git a/rott/rt_menu.c b/rott/rt_menu.c
index 61bbe88..9f767f9 100644
--- a/rott/rt_menu.c
+++ b/rott/rt_menu.c
@@ -1536,7 +1536,7 @@ void SetUpControlPanel (void)

       if ( consoleplayer != 0 )
          {
-         MainMenu[battlemode].routine = ( void (*)(int) )BattleGamePlayerSetup;
+         MainMenu[battlemode].routine = (menuptr)BattleGamePlayerSetup;
          }
       }
    }
@@ -2291,8 +2291,8 @@ int HandleMenu (CP_iteminfo *item_i, CP_itemtype *items, void (*routine)(int w))
    switch (exit)
    {
       case 1:
-         if ((items+handlewhich)->routine!=NULL)
-            (items+handlewhich)->routine(0);
+         if ((items+handlewhich)->routine.v!=NULL)
+            (items+handlewhich)->routine.vi(0);
          return (handlewhich);

       case 2:
@@ -3154,7 +3154,7 @@ void AdjustMenuStruct

    {
    MainMenu[ savegame ].active         = CP_Inactive;
-   MainMenu[ viewscores ].routine      = ( void * )CP_ViewScores;
+   MainMenu[ viewscores ].routine      = (menuptr)CP_ViewScores;
    MainMenu[ viewscores ].texture[ 6 ] = '7';
    MainMenu[ viewscores ].texture[ 7 ] = '\0';
    MainMenu[ viewscores ].letter       = 'V';
@@ -4996,7 +4996,7 @@ void MenuFixup
    MainMenu[ viewscores ].texture[ 6 ] = '1';
    MainMenu[ viewscores ].texture[ 7 ] = '0';
    MainMenu[ viewscores ].texture[ 8 ] = '\0';
-   MainMenu[ viewscores ].routine      = ( void * )CP_EndGame;
+   MainMenu[ viewscores ].routine      = (menuptr)CP_EndGame;
    MainMenu[ viewscores ].letter       = 'E';
    strcpy (MainMenuNames[ viewscores ] , "END GAME");
    MainMenu[ savegame ].active         = CP_Active;
@@ -6335,7 +6335,7 @@ void DrawTimeLimitOptionDescription( int w )

 #define TURN_OFF_BATTLE_MODE( x ) \
    ModeMenu[ ( x ) - 1 ].active  = CP_SemiActive; \
-   ModeMenu[ ( x ) - 1 ].routine = NULL;
+   ModeMenu[ ( x ) - 1 ].routine = (menuptr)NULL;

 //****************************************************************************
diff --git a/rott/rt_menu.h b/rott/rt_menu.h
index 0fa9c39..03a2eaa 100644
--- a/rott/rt_menu.h
+++ b/rott/rt_menu.h
@@ -64,14 +64,21 @@ typedef struct
    } CP_iteminfo;

-typedef void (*menuptr)(int);
+typedef union
+{
+  void (*v);
+  void (*vv)(void);
+  void (*vi)(int); // CP_Quit()
+  int (*iv)(void); // CP_SaveGame()
+  int (*iii)(int, int); // CP_LoadGame()
+} menuptr;

 typedef struct
 {
    int active;
    char texture[9];
    char letter;
-   void (* routine)(int temp1);
+   menuptr routine;
 } CP_itemtype;

 enum