linux-kernel - Re: [Linux-fbdev-devel] [PATCH 7/9] viafb: VIA Frame Buffer Device Driver

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20080508074134.GB11121@localhost.localdomain>
Date:	Thu, 8 May 2008 09:41:34 +0200
From:	Ondrej Zajicek <santiago@...reenet.org>
To:	JosephChan@....com.tw
Cc:	akpm@...ux-foundation.org, geert@...ux-m68k.org,
	linux-fbdev-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org
Subject: Re: [Linux-fbdev-devel] [PATCH 7/9] viafb: VIA Frame Buffer Device Driver

On Wed, May 07, 2008 at 07:09:44PM +0800, JosephChan@....com.tw wrote:
> +static int viafb_ioctl(struct fb_info *info, u_int cmd, u_long arg)
> +{
> +	switch (cmd) {
> +	case VIAFB_GET_CHIP_INFO:	/*struct chip_information chip_info ; */
> +	if (copy_to_user((void __user *)arg, &chip_info, sizeof(chip_info)))
> +		return -EFAULT;
> +		break;
> +	case VIAFB_GET_INFO_SIZE:
> +		return put_user(sizeof(viainfo), argp);
> +	case VIAFB_GET_INFO:
> +		return ioctl_get_viafb_info(arg);
...
> +	case VIAFB_GET_RESOLUTION:

Some of these ioctls looks like they duplicate standard fbdev ioctls.
What is a reason for that?

> +int get_mode_index(int hres, int vres, int flag)
> +{
> +	DEBUG_MSG(KERN_INFO "get_mode_index!\n");
> +
> +	if (flag == 0) {
> +		if (hres == 480 && vres == 640) {
> +			resMode = VIA_RES_480X640;
> +			mode = "480x640";
> +		} else if (hres == 640 && vres == 480) {
> +			resMode = VIA_RES_640X480;
> +			mode = "640x480";
> +		} else if (hres == 800 && vres == 480) {
> +			resMode = VIA_RES_800X480;
> +			mode = "800x480";
> +		} else if (hres == 800 && vres == 600) {
> +			resMode = VIA_RES_800X600;
> +			mode = "800x600";
> +		} else if (hres == 1024 && vres == 768) {
> +			resMode = VIA_RES_1024X768;
> +			mode = "1024x768";
...

This is particularly ugly function. Some loop through a table is perhaps
better than 250 lines of such if statements, isn't it?

> +		if (!strncmp(this_opt, "mode=", 5)) {
> +			mode = kmalloc(strlen(this_opt + 4), GFP_KERNEL);
> +			strcpy(mode, this_opt + 5);
> +		} else if (!strncmp(this_opt, "mode1=", 6)) {
> +			mode1 = kmalloc(strlen(this_opt + 5), GFP_KERNEL);
> +			strcpy(mode1, this_opt + 6);
> +		} else if (!strncmp(this_opt, "bpp=", 4)) {
> +			via_fb_bpp = simple_strtoul(this_opt + 4, NULL, 0);
> +		} else if (!strncmp(this_opt, "bpp1=", 5)) {
> +			via_fb_bpp1 = simple_strtoul(this_opt + 5, NULL, 0);
> +		} else if (!strncmp(this_opt, "refresh=", 8)) {
> +			refresh = simple_strtoul(this_opt + 8, NULL, 0);
> +		} else if (!strncmp(this_opt, "refresh1=", 9)) {
> +			refresh1 = simple_strtoul(this_opt + 9, NULL, 0);

Most drivers accept default mode specification like 1024x768-16@60
(see Documentation/fb/modedb.txt). Is there a reason to have separate
bpp= and refresh= ?

-- 
Elen sila lumenn' omentielvo

Ondrej 'SanTiago' Zajicek (email: santiago@...reenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."
--
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/