Re: [PATCH 10/13] eeepc-laptop: compare proper return values in get_cpufv

[Darren Hart]

On Sat, Sep 13, 2014 at 01:06:49AM +0200, Frans Klaver wrote:
> In get_cpufv the return value of get_acpi is stored in the cpufv struct.
> Right before this value is checked for errors, it is and'ed with 0xff.
> This means c->cur can never be less than zero. Besides that, the actual
> error value is ignored.
> 
> c->num is also and'ed with 0xff, which means we can ignore values below
> zero.
> 
> Check the result of get_acpi() right away. While at it, propagate the
> error if we got one.
> 
> Signed-off-by: Frans Klaver <fransklaver@xxxxxxxxx>
> ---
>  drivers/platform/x86/eeepc-laptop.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
> index 47488d3..828db56 100644
> --- a/drivers/platform/x86/eeepc-laptop.c
> +++ b/drivers/platform/x86/eeepc-laptop.c
> @@ -332,9 +332,12 @@ struct eeepc_cpufv {
>  static int get_cpufv(struct eeepc_laptop *eeepc, struct eeepc_cpufv *c)
>  {
>  	c->cur = get_acpi(eeepc, CM_ASL_CPUFV);
> +	if (c->cur < 0)
> +		return c->cur;
> +
>  	c->num = (c->cur >> 8) & 0xff;
>  	c->cur &= 0xff;
> -	if (c->cur < 0 || c->num <= 0 || c->num > 12)
> +	if (c->num == 0 || c->num > 12)
>  		return -ENODEV;
>  	return 0;

This patch is fine as is. However, Greg has supported propogating the error code
through to the sysfs interface (if I understand him correctly on an earlier post
to this list). This would require an addition change to this patch would
propogated the get_cpufv error code in show_available_cpuv(), show_cpuv(), and
store_cpuv(). As it is, we return -ENODEV on any failure, where an ACPI call
error should probably return -ENXIO as I understand it.

However, there was a rather famous change in error code handling in which pulse
audio broke and Linus was very upset with one of his maintainers.

How do we know when it is acceptable to change which error code is returned?

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