Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for some compiler warnings #17

Open
srett opened this issue Jul 27, 2017 · 0 comments
Open

Fixes for some compiler warnings #17

srett opened this issue Jul 27, 2017 · 0 comments

Comments

@srett
Copy link

srett commented Jul 27, 2017

I tried compiling with clang (linux64) and cranked up the warnings, which uncovered a few potential bugs, but as I'm not familiar with the code base I cannot tell for sure how to properly fix them. ;-)

So here we go:

diff --git a/src/jnstub.c b/src/jnstub.c
index d547d8d..9d2a33b 100644
--- a/src/jnstub.c
+++ b/src/jnstub.c
@@ -1401,7 +1401,7 @@ MoreKeys(short searchstat, short searchwall, short searchsector, short pointhigh
 
 
 
-    if (KEY_PRESSED(KEYSC_RALT + 128) && KEY_PRESSED(KEYSC_RCTRL + 128))
+    if (KEY_PRESSED(KEYSC_RALT /* + 128 */) && KEY_PRESSED(KEYSC_RCTRL /* + 128 */))
         {
         if (KEY_PRESSED(KEYSC_KPMINUS))
             {
@@ -2736,6 +2736,10 @@ DrawClipBox(short spritenum)
         x = mulscale14(x - posx, zoom);
         y = mulscale14(y - posy, zoom);
         }
+    else
+        {
+        x = y = 0;
+        }
 
     x += 320;
     y += 200;

The KEY_PRESSED part is a macro that eventually results in an array access. The array has 256 elements, but because of +128 an OOB access occurs (KEYSC_RALT == 184, KEYSC_RCTRL == 157). I didn't understand right away what the +128 is supposed to achieve, so just removing it is probably wrong.
The second change just fixes uninitialized variable access.

diff --git a/src/jplayer.c b/src/jplayer.c
index b2e21a1..623b117 100644
--- a/src/jplayer.c
+++ b/src/jplayer.c
@@ -651,7 +651,7 @@ void computergetinput(int snum, SW_PACKET *syn)
         // Below formula fails in certain cases
         //syn->angvel = min(max((((daang+1024-damyang)&2047)-1024)>>1,-MAXANGVEL),MAXANGVEL); //was 127
         p->pang = daang;
-        syn->aimvel = min(max((zang-p->horiz)>>1,-PLAYER_HORIZ_MAX),PLAYER_HORIZ_MAX);
+        syn->aimvel = min(max((zang-p->horiz)>>1,-MAXANGVEL),MAXANGVEL);
         // Sets type of aiming, auto aim for bots
         syn->bits |= (1<<SK_AUTO_AIM);
         return;

syn->aimvel is a signed char, but PLAYER_HORIZ_MAX is 299, so it gets truncated to 43. Not sure if this was intended, also not sure if MAXANVGEL is the proper choice here. Fun fact: MAXANGVEL is defined 100 in player.c and game.c, but 80 in jplayer.c.

I also found a few signed/unsigned mismatches in jfaudiolib - are you interested in patches for those? My initial reason for digging around was that sound is pretty broken on Linux, most sounds are very very quiet while some effects play at normal volume. No luck so far tracking it down though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant