From: =?utf-8?Q?Bj=C3=B8rn_Mork?= <bjorn@mork.no>
To: Adam Lee <adam.lee@canonical.com>
Cc: linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org,
        ibm-acpi-devel@lists.sourceforge.net,
        Matthew Garrett <matthew.garrett@nebula.com>,
        Henrique de Moraes Holschuh <ibm-acpi@hmh.eng.br>,
        Alex Hung <alex.hung@canonical.com>
Subject: Re: [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs
Organization: m
References: <1370593209-21358-1-git-send-email-adam.lee@canonical.com>
	<1370593209-21358-2-git-send-email-adam.lee@canonical.com>
Date: Fri, 07 Jun 2013 10:53:50 +0200
In-Reply-To: <1370593209-21358-2-git-send-email-adam.lee@canonical.com> (Adam
	Lee's message of "Fri, 7 Jun 2013 16:20:07 +0800")
Message-ID: <87vc5qxpwh.fsf@nemi.mork.no>
User-Agent: Gnus/5.11002 (No Gnus v0.20) Emacs/23.4 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 886
Lines: 29

Adam Lee <adam.lee@canonical.com> writes:

> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -5401,9 +5401,12 @@ static int led_write(char *buf)
>  		return -ENODEV;
>  
>  	while ((cmd = next_cmd(&buf))) {
> -		if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > 15)
> +		if (sscanf(cmd, "%d", &led) != 1)
>  			return -EINVAL;
>  
> +		if (!tpacpi_leds[led].led)
> +			return -ENODEV;

This looks risky.  Why did you remove the index sanity check?  What will
happen now if the input is e.g "-1" or "42"?

BTW, the magic number 15 should probably be (TPACPI_LED_NUMLEDS - 1)
instead.


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