Re: [PATCH] platinumfb: misplaced parenthesis

["Ville Syrjälä" <syrjala@sci.fi>]

On Fri, May 15, 2009 at 10:43:51PM +0200, Roel Kluin wrote:
> Since `+' has a higher precedence than the trinary operator `?', this
> added `hres * (1 << color_mode)' to the boolean, testing videomode
> and depth.
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> Sorry for the dup. this title and changelog should be better.
> 
> > 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));

How about just pulling that magic video_mode/cmode handling out of that
single statement so that it could be parsed with less effort?

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

------------------------------------------------------------------------------
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