Re: [PATCH] qlogicpti: Add missing parentheses

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 18 Feb 2009 14:39:16 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Roel Kluin <roel.kluin@gmail.com>
> Date: Wed, 18 Feb 2009 13:48:11 +0100
> 
> > Add missing parentheses
> > 
> > Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> 
> So what bug does this cause?
> 
> >  #define QLOGICPTI_REQ_QUEUE_LEN	255	/* must be power of two - 1 */
> > -#define QLOGICPTI_MAX_SG(ql)	(4 + ((ql) > 0) ? 7*((ql) - 1) : 0)
> > +#define QLOGICPTI_MAX_SG(ql)	(4 + (((ql) > 0) ? 7*((ql) - 1) : 0))
> 
> The "ql > 0" bids to the "?" operator, so this expression
> does the right thing as-is as far as I can tell.
> 
> Do you think that "4 + ((ql > 0)" binds to "?", I don't think
> it does.

The code is crap, and buggy if passed an expression with side-effects. 
Let's turn it into the way it _should_ have been written.

yes, "+" has higher precedence than ?:, so the current code is:

static inline int qlogicpti_max_sg(int ql)
{
	if (4 + (ql > 0))
		return 7 * (ql - 1);
	else
		return 0;
}

Now, 4 plus a boolean can only ever evaluate to 4 or 5, so this
function can never return zero.  So yeah, I assume that this was meant:

static inline int qlogicpti_max_sg(int ql)
{
	if (ql > 0)
		return 7 * (ql - 1) + 4;
	else
		return 4;
}