Thread (10 messages) 10 messages, 5 authors, 2009-05-18

Re: [PATCH] staging: misplaced parentheses?

From: Roel Kluin <hidden>
Date: 2009-05-15 10:52:47
Subsystem: framebuffer layer, the rest · Maintainers: Helge Deller, Linus Torvalds

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 <redacted>
---
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help