linux-kernel - RE: [PATCH 07/20] video: msm: Allow users to request a larger x and y virtual fb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <94462146d712916438b435acb20c9f00.squirrel@www.codeaurora.org>
Date:	Mon, 21 Mar 2011 17:21:10 -0700 (PDT)
From:	"Carl Vanderlip" <carlv@...eaurora.org>
To:	"Janorkar, Mayuresh" <mayur@...com>
Cc:	"Carl Vanderlip" <carlv@...eaurora.org>,
	"David Brown" <davidb@...eaurora.org>,
	"Daniel Walker" <dwalker@...o99.com>,
	"Bryan Huntsman" <bryanh@...eaurora.org>,
	"Brian Swetland" <swetland@...gle.com>,
	"Dima Zavin" <dima@...roid.com>,
	"Rebecca Schultz Zavin" <rebecca@...roid.com>,
	"Colin Cross" <ccross@...roid.com>,
	"linux-fbdev@...r.kernel.org" <linux-fbdev@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 07/20] video: msm: Allow users to request a larger x 
     and y virtual fb

>> diff --git a/drivers/video/msm/msm_fb.c b/drivers/video/msm/msm_fb.c
>> index 04d0067..6af8b41 100644
>> --- a/drivers/video/msm/msm_fb.c
>> +++ b/drivers/video/msm/msm_fb.c

>> @@ -323,14 +327,46 @@ error:
>>
>>  static int msmfb_check_var(struct fb_var_screeninfo *var, struct
>> fb_info
>> *info)
>>  {
>> +	u32 size;
>> +
>>  	if ((var->xres != info->var.xres) ||
>>  	    (var->yres != info->var.yres) ||
>> -	    (var->xres_virtual != info->var.xres_virtual) ||
>> -	    (var->yres_virtual != info->var.yres_virtual) ||
>>  	    (var->xoffset != info->var.xoffset) ||
>>  	    (var->bits_per_pixel != info->var.bits_per_pixel) ||
>>  	    (var->grayscale != info->var.grayscale))
>>  		 return -EINVAL;
>> +
>> +	size = var->xres_virtual * var->yres_virtual *
>> +		(var->bits_per_pixel >> 3);
>
> How about defining a new macro BYTES_PER_PIXEL_VAR for fb_var_screeninfo
> also? That would make code more readable.
>
>> +	if (size > info->fix.smem_len)
>> +		return -EINVAL;
>> +	return 0;
>> +}

Name might be a little easy to confuse with the other BYTES_PER_PIXEL, but
overall readability would increase IMHO; Done.

>> +static int msmfb_set_par(struct fb_info *info)
>> +{
>> +	struct fb_var_screeninfo *var = &info->var;
>> +	struct fb_fix_screeninfo *fix = &info->fix;
>> +
>> +	/* we only support RGB ordering for now */
>> +	if (var->bits_per_pixel == 32 || var->bits_per_pixel == 24) {
>> +		var->red.offset = 0;
>> +		var->red.length = 8;
>> +		var->green.offset = 8;
>> +		var->green.length = 8;
>> +		var->blue.offset = 16;
>> +		var->blue.length = 8;
>
> var->red is a fb_bitfield variable.
> It provides offset, length and msb_right.
>
> struct fb_bitfield {
>     __u32 offset;                   /* beginning of bitfield        */
>     __u32 length;                   /* length of bitfield           */
>     __u32 msb_right;                /* != 0 : Most significant bit is */
>                                     /* right */
> }
> Please don't keep msb_right unassigned.
>
>> +	} else if (var->bits_per_pixel == 16) {
>> +		var->red.offset = 11;
>> +		var->red.length = 5;
>> +		var->green.offset = 5;
>> +		var->green.length = 6;
>> +		var->blue.offset = 0;
>> +		var->blue.length = 5;
>> +	} else
>> +		return -1;
>
> Please use standard error code provided by Linux kernel -EINVAL
> (-22 Invalid argument)
>
>> +	fix->line_length = var->xres * var->bits_per_pixel / 8;
> Why to divide by 8? Atleast use >>3, bitwise operations that would take
> less cpu cycles)
> As I stated earlier define a new macro for var also.
>
>> +
>>  	return 0;
>>  }

And Done. And done again... and while I'm at it, all the changes you
suggested are being pulled into v2 (except for updating Google's copyright
date (see: Brian Swetland's response)).

Thank you for reviewing my patches :)

   -Carl V.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/