Messages in this thread Patch in this message | | | Date | Tue, 27 Nov 2001 11:38:30 -0700 | From | Andreas Dilger <> | Subject | Re: [BUG] Bad #define, nonportable C, missing {} |
| |
On Nov 27, 2001 16:03 -0200, vda wrote: > On Monday 26 November 2001 18:28, Alan Cox wrote: > > Nothing to do with me 8). I didnt write that bit of the i2o code. I agree > > its both confusing and buggy. Send a fix ? > > This is a test to be sure my replacement is equivalent: > -------------------- > #include <stdio.h> > #define MODINC(x,y) (x = x++ % y) > #define MODULO_INC(x,y) ((x) = ((x)%(y))+1)
Ugh, clearly the code is broken, so we don't want equivalent code, but correct code. Use the unambiguous "MODINC(x,y) ((x) = ((x) + 1) % (y))" form and not "have a bug that has properly defined behaviour under ANSI C".
Just looking at the code, it is fairly clear that the desire is to keep q_in and q_out >= 0 and < I20_EVT_Q_LEN, which is the size of the event_q array. With the buggy version, it is possible that you could have q_in or q_out == I2O_EVT_Q_LEN, which is overflowing the array. Bad, bad, bad.
--- i2o_config.c.new Mon Oct 22 13:39:56 2001 +++ i2o_config.c.orig Tue Nov 27 16:03:19 2001 @@ -45,7 +45,7 @@ static spinlock_t i2o_config_lock = SPIN_LOCK_UNLOCKED; struct wait_queue *i2o_wait_queue;
-#define MODINC(x,y) (x = x++ % y) +#define MODINC(x,y) ((x) = ((x) + 1) % (y))
struct i2o_cfg_info { Cheers, Andreas -- Andreas Dilger http://sourceforge.net/projects/ext2resize/ http://www-mddsp.enel.ucalgary.ca/People/adilger/
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |