Re: [PATCH] staging: misplaced parentheses?

[Roel Kluin <roel.kluin@gmail.com>]

The leftmost `+' has a higher precedence than the `?' so this returns
(vres * (hres * (1 << color_mode) + (VMODE && CMODE) ? 0x10 : 0x20) + 0x1000

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---

> Yes, the original code looks ok and the patches one not quite ...

Ok, my former patch may have been wrong, but unless I am very much
mistaken, there is something fishy here.

> The idea is that the framebuffer is offset by 0x1020 normally, except
> when the video mode is VMODE_832_624_75 and the depth > 8 in which case
> it's offet by 0x1010. It's a weird piece of HW but I think the original
> code is correct. Or do I miss something ?

Thanks for this, let's simplify to explain what I think that goes wrong:

printf("%d\n", 1 + 0 ? 2 : 4);

This prints `2': since the `+' operator has a higher precedence than the
trinary `?' operator.

In the original code we had:

return vmode_attrs[video_mode-1].vres *
  	       (vmode_attrs[video_mode-1].hres * (1<<color_mode) +
  		((video_mode == VMODE_832_624_75) &&
		 (color_mode > CMODE_8)) ? 0x10 : 0x20) + 0x1000;

note that `hres * (1<<color_mode)' is added to
((video_mode == VMODE_832_624_75) && (color_mode > CMODE_8))
before the trinary operator `?' occurs.

So maybe instead of the first patch you may want to apply this?

diff --git a/drivers/video/platinumfb.c b/drivers/video/platinumfb.c
index 03b3670..2cc986f 100644
--- a/drivers/video/platinumfb.c
+++ b/drivers/video/platinumfb.c
@@ -224,7 +224,7 @@ static inline int platinum_vram_reqd(int video_mode, int color_mode)
 	return vmode_attrs[video_mode-1].vres *
 	       (vmode_attrs[video_mode-1].hres * (1<<color_mode) +
 		((video_mode == VMODE_832_624_75) &&
-		 (color_mode > CMODE_8)) ? 0x10 : 0x20) + 0x1000;
+		 (color_mode > CMODE_8) ? 0x1010 : 0x1020));
 }
 
 #define STORE_D2(a, d) { \

------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables 
unlimited royalty-free distribution of the report engine 
for externally facing server and web deployment. 
http://p.sf.net/sfu/businessobjects