Subject: bin/9698: Operator precedence errors in phantasia
To: None <gnats-bugs@gnats.netbsd.org>
From: None <John.P.Darrow@wheaton.edu>
List: netbsd-bugs
Date: 03/28/2000 18:03:36
>Number:         9698
>Category:       bin
>Synopsis:       Operator precedence errors in phantasia
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people (Utility Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Mar 28 18:03:00 2000
>Last-Modified:
>Originator:     John Darrow
>Organization:
	Again on my own time :)
>Release:        all
>Environment:
System: NetBSD jdarrowpiii.wheaton.edu 1.4V NetBSD 1.4V (JDARROW) #0: Tue Mar 21 15:04:28 CST 2000 jdarrow@jdarrowpiii.wheaton.edu:/var/src/sys/arch/i386/compile/JDARROW i386


>Description:
	src/games/phantasia/fight.c contains two uses of a statement
	of the form a = b + (conditional) ? c : d .  In each of these
	cases, documentation and common sense show that the intended
	result is a = b + ((conditional) ? c : d) .

>How-To-Repeat:
	Notice that a Jabberwock always summons a Fir Darrig when it
	runs away, instead of a Jubjub Bird or Bandersnatch.  Examine
	code, see error, search for similar errors.
>Fix:
	The following is the "easy" way to fix this, simply adding parens:
Index: /source/NetBSD-current/src/games/phantasia/fight.c
===================================================================
RCS file: /source/cvs/netbsd/current/src/games/phantasia/fight.c,v
retrieving revision 1.1.1.2
retrieving revision 1.2
diff -u -r1.1.1.2 -r1.2
--- fight.c	1999/09/09 16:22:47	1.1.1.2
+++ fight.c	2000/03/29 01:29:34	1.2
@@ -549,7 +549,7 @@
 			mvprintw(Lines++, 0,
 			    "%s flew away, and left you to contend with one of its friends.",
 			    Enemyname);
-			Whichmonster = 55 + (drandom() > 0.5) ? 22 : 0;
+			Whichmonster = 55 + ((drandom() > 0.5) ? 22 : 0);
 			longjmp(Fightenv, 0);
 			/* NOTREACHED */
 
@@ -869,7 +869,7 @@
 			Curmonster.m_energy = Player.p_might * 30.0;
 			Curmonster.m_type = SM_MORGOTH;
 			Curmonster.m_speed = Player.p_speed * 1.1
-			    + (Player.p_specialtype == SC_EXVALAR) ? Player.p_speed : 0.0;
+			    + ((Player.p_specialtype == SC_EXVALAR) ? Player.p_speed : 0.0);
 			Curmonster.m_flock = 0.0;
 			Curmonster.m_treasuretype = 0;
 			Curmonster.m_experience = 0.0;


	However, a little examination shows that each statement could be
	written more simply, thus eliminating most of the precedence
	confusion, as follows:

Index: /var/src/games/phantasia/fight.c
===================================================================
RCS file: /source/cvs/netbsd/current/src/games/phantasia/fight.c,v
retrieving revision 1.1.1.2
diff -u -r1.1.1.2 fight.c
--- fight.c	1999/09/09 16:22:47	1.1.1.2
+++ fight.c	2000/03/29 01:47:55
@@ -549,7 +549,7 @@
 			mvprintw(Lines++, 0,
 			    "%s flew away, and left you to contend with one of its friends.",
 			    Enemyname);
-			Whichmonster = 55 + (drandom() > 0.5) ? 22 : 0;
+			Whichmonster = (drandom() > 0.5) ? 77 : 55;
 			longjmp(Fightenv, 0);
 			/* NOTREACHED */
 
@@ -868,8 +868,8 @@
 			Curmonster.m_brains = Player.p_brains;
 			Curmonster.m_energy = Player.p_might * 30.0;
 			Curmonster.m_type = SM_MORGOTH;
-			Curmonster.m_speed = Player.p_speed * 1.1
-			    + (Player.p_specialtype == SC_EXVALAR) ? Player.p_speed : 0.0;
+			Curmonster.m_speed = Player.p_speed
+			    * ((Player.p_specialtype == SC_EXVALAR) ? 2.1 : 1.1);
 			Curmonster.m_flock = 0.0;
 			Curmonster.m_treasuretype = 0;
 			Curmonster.m_experience = 0.0;


	Take your pick as to which one you want to use.  (Not that
	the rest of the code in phantasia is all that pretty :)
>Audit-Trail:
>Unformatted: