linux-kernel - Re: [PATCH] Samsung backlight 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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20090814222533.26D94526EC9@mailhub.coreip.homeip.net>
Date:	Fri, 14 Aug 2009 14:54:40 -0700
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Greg KH <greg@...ah.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Samsung backlight driver

Hi Greg,

On Fri, Aug 14, 2009 at 12:48:15PM -0700, Greg KH wrote:
> +
> +static int update_status(struct backlight_device *bd)
> +{
> +	if (!pci_device)
> +		return -ENODEV;
> +	if (!backlight_device)

How is this possible?

> +		return -ENODEV;
> +
> +	current_brightness = bd->props.brightness;
> +	set_brightness();
> +	return 0;
> +}
> +
> +static struct backlight_ops backlight_ops = {
> +	.get_brightness	= get_brightness,
> +	.update_status	= update_status,
> +};
> +
> +static int find_video_card(void)
> +{
> +	struct pci_dev *dev = NULL;
> +
> +	while ((dev = pci_get_device(0x8086, 0x27ae, dev)) != NULL) {
> +		/* Found one, so let's save it off */
> +		if (!pci_device)
> +			pci_device = pci_dev_get(dev);

If you need to iterate all matching devices you need to save them all or
release references to "extras" right here... Or, better yet, just break
after you find the first one.

> +	}
> +
> +	if (!pci_device)
> +		return 0;

Why not -ENODEV? What is the point of having this driver loaded if the
device is not available? Then you could drop all silly "if (!pci_device)"
checks.

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