On Wed, 18 Feb 2009 14:39:16 -0800 (PST)
David Miller <davem@xxxxxxxxxxxxx> wrote:
> From: Roel Kluin <roel.kluin@xxxxxxxxx>
> Date: Wed, 18 Feb 2009 13:48:11 +0100
>
> > Add missing parentheses
> >
> > Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx>
>
> 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;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
