When an error occurs in sysfs show callback, we should return the errno
directly instead of formatting it as the result, which produces
meaningless output and doesn't inform the userspace of the error.

Fixes: 468f96bfa3a0 ("platform/x86: panasonic-laptop: Add support for battery charging threshold (eco mode)")
Fixes: d5a81d8e864b ("platform/x86: panasonic-laptop: Add support for optical driver power in Y and W series")
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/platform/x86/panasonic-laptop.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index 2bf94d0ab324..22ca70eb8227 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -614,8 +614,7 @@ static ssize_t eco_mode_show(struct device *dev, struct device_attribute *attr,
 		result = 1;
 		break;
 	default:
-		result = -EIO;
-		break;
+		return -EIO;
 	}
 	return sysfs_emit(buf, "%u\n", result);
 }
@@ -761,7 +760,12 @@ static ssize_t current_brightness_store(struct device *dev, struct device_attrib
 static ssize_t cdpower_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
 {
-	return sysfs_emit(buf, "%d\n", get_optd_power_state());
+	int state = get_optd_power_state();
+
+	if (state < 0)
+		return state;
+
+	return sysfs_emit(buf, "%d\n", state);
 }
 
 static ssize_t cdpower_store(struct device *dev, struct device_attribute *attr,
-- 
2.47.0

On Mon, 18 Nov 2024 06:46:39 +0000, Yao Zi wrote:

> When an error occurs in sysfs show callback, we should return the errno
> directly instead of formatting it as the result, which produces
> meaningless output and doesn't inform the userspace of the error.
> 
> 


Thank you for your contribution, it has been applied to my local
review-ilpo branch. Note it will show up in the public
platform-drivers-x86/review-ilpo branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/1] platform/x86: panasonic-laptop: Return errno correctly in show callback
      commit: 5c7bebc1a3f0661db558d60e14dde27fc216d9dc

--
 i.

Hi,

On 18-Nov-24 7:46 AM, Yao Zi wrote:
> When an error occurs in sysfs show callback, we should return the errno
> directly instead of formatting it as the result, which produces
> meaningless output and doesn't inform the userspace of the error.
> 
> Fixes: 468f96bfa3a0 ("platform/x86: panasonic-laptop: Add support for battery charging threshold (eco mode)")
> Fixes: d5a81d8e864b ("platform/x86: panasonic-laptop: Add support for optical driver power in Y and W series")
> Signed-off-by: Yao Zi <ziyao@disroot.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/platform/x86/panasonic-laptop.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
> index 2bf94d0ab324..22ca70eb8227 100644
> --- a/drivers/platform/x86/panasonic-laptop.c
> +++ b/drivers/platform/x86/panasonic-laptop.c
> @@ -614,8 +614,7 @@ static ssize_t eco_mode_show(struct device *dev, struct device_attribute *attr,
>  		result = 1;
>  		break;
>  	default:
> -		result = -EIO;
> -		break;
> +		return -EIO;
>  	}
>  	return sysfs_emit(buf, "%u\n", result);
>  }
> @@ -761,7 +760,12 @@ static ssize_t current_brightness_store(struct device *dev, struct device_attrib
>  static ssize_t cdpower_show(struct device *dev, struct device_attribute *attr,
>  			    char *buf)
>  {
> -	return sysfs_emit(buf, "%d\n", get_optd_power_state());
> +	int state = get_optd_power_state();
> +
> +	if (state < 0)
> +		return state;
> +
> +	return sysfs_emit(buf, "%d\n", state);
>  }
>  
>  static ssize_t cdpower_store(struct device *dev, struct device_attribute *attr,