fix many clang default warnings, some of them being code logic errors

[bubbleguuum]

Here are the clang warnings fixed (here listed as errors as I used -Werror while addressing them one by one):

src/engine/md5.c:157:31: error: 'memset' call operates on objects of type 'md5_context_t' (aka 'struct md5_context_s') while the size is based on a different type 'md5_context_t *' (aka 'struct md5_context_s *') [-Werror,-Wsizeof-pointer-memaccess]
  157 |         memset(ctx, 0, sizeof(ctx));    /* In case it's sensitive */

src/engine/con_console.c:127:7: error: address of array 'console_linebuffer' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  127 |         if (!console_linebuffer) {
      |             ~^~~~~~~~~~~~~~~~~~
src/engine/con_console.c:179:7: error: address of array 'console_linebuffer' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  179 |         if (!console_linebuffer) {
      |             ~^~~~~~~~~~~~~~~~~~
src/engine/con_console.c:474:7: error: address of array 'console_linebuffer' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  474 |         if (!console_linebuffer) {
      |             ~^~~~~~~~~~~~~~~~~~

src/engine/d_main.c:140:15: error: multiple unsequenced modifications to 'eventhead' [-Werror,-Wunsequenced]
  140 |         eventhead = (++eventhead) & (MAXEVENTS - 1);
      |                   ~  ^
src/engine/d_main.c:151:46: error: multiple unsequenced modifications to 'eventtail' [-Werror,-Wunsequenced]
  151 |         for (; eventtail != eventhead; eventtail = (++eventtail) & (MAXEVENTS - 1)) {

src/engine/g_actions.c:246:17: error: using the result of an assignment as a condition without parentheses [-Werror,-Wparentheses]
  246 |                 else if (cvar = CON_CvarGet(al->cmd)) {
      |                          ~~~~~^~~~~~~~~~~~~~~~~~~~~~
src/engine/g_actions.c:246:17: note: place parentheses around the assignment to silence this warning
  246 |                 else if (cvar = CON_CvarGet(al->cmd)) {
      |                               ^                     
      |                          (                          )

src/engine/m_menu.c:1964:29: error: array index 5 is past the end of the array (that has type 'const float[5]') [-Werror,-Warray-bounds]
 1964 |         else if (dfcmp(checkratio, ratioVal[5])) {
      |                                    ^        ~
src/engine/m_menu.c:1912:1: note: array 'ratioVal' declared here
 1912 | static const float ratioVal[5] = {
      | ^
src/engine/m_menu.c:2189:36: error: multiple unsequenced modifications to 'm_aspectRatio' [-Werror,-Wunsequenced]
 2189 |                 m_aspectRatio = MAX(m_aspectRatio--, 0);
      |                               ~                  ^
src/engine/doomtype.h:38:28: note: expanded from macro 'MAX'
   38 | #define MAX(a,b) ((a)>(b)?(a):(b))
      |                            ^
src/engine/m_menu.c:2247:34: error: multiple unsequenced modifications to 'm_ScreenSize' [-Werror,-Wunsequenced]
 2247 |                 m_ScreenSize = MAX(m_ScreenSize--, 0);
      |                              ~                 ^
src/engine/doomtype.h:38:28: note: expanded from macro 'MAX'
   38 | #define MAX(a,b) ((a)>(b)?(a):(b))
      |                            ^
src/engine/m_menu.c:4729:38: error: comparison of array 'currentMenu->menuitems[i].name' not equal to a null pointer is always true [-Werror,-Wtautological-pointer-compare]
 4729 |                 else if (currentMenu->menuitems[i].name != NULL) {
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~    ~~~~
src/engine/m_menu.c:4981:29: error: comparison of array 'currentMenu->menuitems[0].name' not equal to a null pointer is always true [-Werror,-Wtautological-pointer-compare]
 4981 |                 currentMenu->menuitems[0].name != NULL) {

src/engine/st_stuff.c:422:35: error: multiple unsequenced modifications to 'st_msgalpha' [-Werror,-Wunsequenced]
  422 |                         st_msgalpha = MAX((st_msgalpha -= ST_MSGFADETIME), 0);
      |                                     ~                  ^
src/engine/doomtype.h:38:28: note: expanded from macro 'MAX'
   38 | #define MAX(a,b) ((a)>(b)?(a):(b))

[bubbleguuum]

There are also these 2 warnings that I did not address yet:

src/engine/net_client.c:1279:19: error: implicit conversion from enumeration type 'net_connstate_t' to different enumeration type 'net_clientstate_t' [-Werror,-Wenum-conversion]
 1279 |                         client_state = NET_CONN_STATE_DISCONNECTED;
      |                                      ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~

it looks to me that the line should be client_connection.state = NET_CONN_STATE_DISCONNECTED;

//////////

src/engine/p_inter.c:1153:38: error: & has lower precedence than >; > will be evaluated first [-Werror,-Wparentheses]
 1153 |                 || (source->flags ^ target->flags) & p_disable_monster_infighting.value > 0))
      |                                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

As it is written, code above is equivalent to (source->flags ^ target->flags) & (p_disable_monster_infighting.value > 0)) Is it correct or should it be ((source->flags ^ target->flags) & p_disable_monster_infighting.value) > 0 ?

[Wolf3s]

It´s ((source->flags ^ target->flags) & p_disable_monster_infighting.value) > 0

[bubbleguuum]

Thanks, fixed the test.

[bubbleguuum]

@Wolf3s

I do not think that the 'fixed wrong test' patch is correct as gcc now gives an error (not a warning!) and compilation fails on the line:

src/engine/p_inter.c: In function ‘P_DamageMobj’:
src/engine/p_inter.c:1153:53: error: invalid operands to binary & (have ‘int’ and ‘float’)
 1153 |                 || ((source->flags ^ target->flags) & p_disable_monster_infighting.value) > 0))
      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                    |                                              |
      |                                    int                                            float

so & is applied between int and float and it does not look right. Interestingly, clang compile this without error.

Would not be surprised the correct statement is || ((source->flags ^ target->flags) && p_disable_monster_infighting.value > 0)

[Wolf3s]

@Wolf3s

I do not think that the 'fixed wrong test' patch is correct as gcc now gives an error (not a warning!) and compilation fails on the line:

src/engine/p_inter.c: In function ‘P_DamageMobj’:
src/engine/p_inter.c:1153:53: error: invalid operands to binary & (have ‘int’ and ‘float’)
 1153 |                 || ((source->flags ^ target->flags) & p_disable_monster_infighting.value) > 0))
      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                    |                                              |
      |                                    int                                            float

so & is applied between int and float and it does not look right. Interestingly, clang compile this without error.

Would not be surprised the correct statement is || ((source->flags ^ target->flags) && p_disable_monster_infighting.value > 0)

use and(&&) and should work.

[bubbleguuum]

When compiling with gcc and LTO enabled (-flto) which might be in default CFLAGS when compiling for some distros, there are these warnings at link time with 3 functions with prototypes not matching. This look a bit suspicious:

src/engine/p_map.c:710:6: warning: type of ‘P_ZMovement’ does not match original declaration [-Wlto-type-mismatch]
  710 | void P_ZMovement(mobj_t* mo, int checkmissile);
      |      ^
src/engine/p_mobj.c:297:6: note: type mismatch in parameter 2
  297 | void P_ZMovement(mobj_t* mo) {
      |      ^
src/engine/p_mobj.c:297:6: note: type ‘void’ should match type ‘int’
src/engine/p_mobj.c:297:6: note: ‘P_ZMovement’ was previously declared here
src/engine/g_game.c:71:13: warning: type of ‘M_SaveGame’ does not match original declaration [-Wlto-type-mismatch]
   71 | extern void M_SaveGame(void);
      |             ^
src/engine/m_menu.c:3202:6: note: type mismatch in parameter 1
 3202 | void M_SaveGame(int choice) {
      |      ^
src/engine/m_menu.c:3202:6: note: type ‘int’ should match type ‘void’
src/engine/m_menu.c:3202:6: note: ‘M_SaveGame’ was previously declared here
src/engine/g_game.c:72:13: warning: type of ‘M_LoadGame’ does not match original declaration [-Wlto-type-mismatch]
   72 | extern void M_LoadGame(void);
      |             ^
src/engine/m_menu.c:3301:6: note: type mismatch in parameter 1
 3301 | void M_LoadGame(int choice) {
      |      ^
src/engine/m_menu.c:3301:6: note: type ‘int’ should match type ‘void’
src/engine/m_menu.c:3301:6: note: ‘M_LoadGame’ was previously declared here

[Wolf3s]

When compiling with gcc and LTO enabled (-flto) which might be in default CFLAGS when compiling for some distros, there are these warnings at link time with 3 functions with prototypes not matching. This look a bit suspicious:

src/engine/p_map.c:710:6: warning: type of ‘P_ZMovement’ does not match original declaration [-Wlto-type-mismatch]
  710 | void P_ZMovement(mobj_t* mo, int checkmissile);
      |      ^
src/engine/p_mobj.c:297:6: note: type mismatch in parameter 2
  297 | void P_ZMovement(mobj_t* mo) {
      |      ^
src/engine/p_mobj.c:297:6: note: type ‘void’ should match type ‘int’
src/engine/p_mobj.c:297:6: note: ‘P_ZMovement’ was previously declared here
src/engine/g_game.c:71:13: warning: type of ‘M_SaveGame’ does not match original declaration [-Wlto-type-mismatch]
   71 | extern void M_SaveGame(void);
      |             ^
src/engine/m_menu.c:3202:6: note: type mismatch in parameter 1
 3202 | void M_SaveGame(int choice) {
      |      ^
src/engine/m_menu.c:3202:6: note: type ‘int’ should match type ‘void’
src/engine/m_menu.c:3202:6: note: ‘M_SaveGame’ was previously declared here
src/engine/g_game.c:72:13: warning: type of ‘M_LoadGame’ does not match original declaration [-Wlto-type-mismatch]
   72 | extern void M_LoadGame(void);
      |             ^
src/engine/m_menu.c:3301:6: note: type mismatch in parameter 1
 3301 | void M_LoadGame(int choice) {
      |      ^
src/engine/m_menu.c:3301:6: note: type ‘int’ should match type ‘void’
src/engine/m_menu.c:3301:6: note: ‘M_LoadGame’ was previously declared here

i see, this could be for another pull request.

[Wolf3s]

type mismatch.

[bubbleguuum]

fixed lto warnings in new PR.

[Wolf3s]

fixed lto warnings in new PR.

merged your request.