2021-05-19 18:04:00

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/1] gpiolib: Use sysfs_emit() in "show" functions

The sysfs_emit() function was introduced to make it less ambiguous
which function is preferred when writing to the output buffer in
a "show" callback [1].

Convert the GPIO library sysfs interface from sprintf() to sysfs_emit()
accordingly, as the latter is aware of the PAGE_SIZE buffer and correctly
returns the number of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/gpio/gpiolib-sysfs.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 7c5afd999210..d836aba91d3c 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -66,9 +66,7 @@ static ssize_t direction_show(struct device *dev,
mutex_lock(&data->mutex);

gpiod_get_direction(desc);
- status = sprintf(buf, "%s\n",
- test_bit(FLAG_IS_OUT, &desc->flags)
- ? "out" : "in");
+ status = sysfs_emit(buf, "%s\n", test_bit(FLAG_IS_OUT, &desc->flags) ? "out" : "in");

mutex_unlock(&data->mutex);

@@ -109,13 +107,9 @@ static ssize_t value_show(struct device *dev,
mutex_lock(&data->mutex);

status = gpiod_get_value_cansleep(desc);
- if (status < 0)
- goto err;
+ if (status >= 0)
+ status = sysfs_emit(buf, "%zd\n", status);

- buf[0] = '0' + status;
- buf[1] = '\n';
- status = 2;
-err:
mutex_unlock(&data->mutex);

return status;
@@ -249,11 +243,11 @@ static ssize_t edge_show(struct device *dev,
mutex_lock(&data->mutex);

for (i = 0; i < ARRAY_SIZE(trigger_types); i++) {
- if (data->irq_flags == trigger_types[i].flags) {
- status = sprintf(buf, "%s\n", trigger_types[i].name);
+ if (data->irq_flags == trigger_types[i].flags)
break;
- }
}
+ if (i < ARRAY_SIZE(trigger_types))
+ status = sysfs_emit(buf, "%s\n", trigger_types[i].name);

mutex_unlock(&data->mutex);

@@ -333,8 +327,7 @@ static ssize_t active_low_show(struct device *dev,

mutex_lock(&data->mutex);

- status = sprintf(buf, "%d\n",
- !!test_bit(FLAG_ACTIVE_LOW, &desc->flags));
+ status = sysfs_emit(buf, "%d\n", !!test_bit(FLAG_ACTIVE_LOW, &desc->flags));

mutex_unlock(&data->mutex);

@@ -412,7 +405,7 @@ static ssize_t base_show(struct device *dev,
{
const struct gpio_chip *chip = dev_get_drvdata(dev);

- return sprintf(buf, "%d\n", chip->base);
+ return sysfs_emit(buf, "%d\n", chip->base);
}
static DEVICE_ATTR_RO(base);

@@ -421,7 +414,7 @@ static ssize_t label_show(struct device *dev,
{
const struct gpio_chip *chip = dev_get_drvdata(dev);

- return sprintf(buf, "%s\n", chip->label ? : "");
+ return sysfs_emit(buf, "%s\n", chip->label ?: "");
}
static DEVICE_ATTR_RO(label);

@@ -430,7 +423,7 @@ static ssize_t ngpio_show(struct device *dev,
{
const struct gpio_chip *chip = dev_get_drvdata(dev);

- return sprintf(buf, "%u\n", chip->ngpio);
+ return sysfs_emit(buf, "%u\n", chip->ngpio);
}
static DEVICE_ATTR_RO(ngpio);

--
2.30.2



2021-05-21 12:59:25

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] gpiolib: Use sysfs_emit() in "show" functions

On Tue, May 18, 2021 at 2:09 PM Andy Shevchenko
<[email protected]> wrote:
>
> The sysfs_emit() function was introduced to make it less ambiguous
> which function is preferred when writing to the output buffer in
> a "show" callback [1].
>
> Convert the GPIO library sysfs interface from sprintf() to sysfs_emit()
> accordingly, as the latter is aware of the PAGE_SIZE buffer and correctly
> returns the number of bytes written into the buffer.
>
> No functional change intended.
>
> [1] Documentation/filesystems/sysfs.rst
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/gpio/gpiolib-sysfs.c | 27 ++++++++++-----------------
> 1 file changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> index 7c5afd999210..d836aba91d3c 100644
> --- a/drivers/gpio/gpiolib-sysfs.c
> +++ b/drivers/gpio/gpiolib-sysfs.c
> @@ -66,9 +66,7 @@ static ssize_t direction_show(struct device *dev,
> mutex_lock(&data->mutex);
>
> gpiod_get_direction(desc);
> - status = sprintf(buf, "%s\n",
> - test_bit(FLAG_IS_OUT, &desc->flags)
> - ? "out" : "in");
> + status = sysfs_emit(buf, "%s\n", test_bit(FLAG_IS_OUT, &desc->flags) ? "out" : "in");
>
> mutex_unlock(&data->mutex);
>
> @@ -109,13 +107,9 @@ static ssize_t value_show(struct device *dev,
> mutex_lock(&data->mutex);
>
> status = gpiod_get_value_cansleep(desc);
> - if (status < 0)
> - goto err;
> + if (status >= 0)
> + status = sysfs_emit(buf, "%zd\n", status);
>
> - buf[0] = '0' + status;
> - buf[1] = '\n';
> - status = 2;
> -err:
> mutex_unlock(&data->mutex);
>
> return status;
> @@ -249,11 +243,11 @@ static ssize_t edge_show(struct device *dev,
> mutex_lock(&data->mutex);
>
> for (i = 0; i < ARRAY_SIZE(trigger_types); i++) {
> - if (data->irq_flags == trigger_types[i].flags) {
> - status = sprintf(buf, "%s\n", trigger_types[i].name);
> + if (data->irq_flags == trigger_types[i].flags)
> break;
> - }
> }
> + if (i < ARRAY_SIZE(trigger_types))
> + status = sysfs_emit(buf, "%s\n", trigger_types[i].name);
>
> mutex_unlock(&data->mutex);
>
> @@ -333,8 +327,7 @@ static ssize_t active_low_show(struct device *dev,
>
> mutex_lock(&data->mutex);
>
> - status = sprintf(buf, "%d\n",
> - !!test_bit(FLAG_ACTIVE_LOW, &desc->flags));
> + status = sysfs_emit(buf, "%d\n", !!test_bit(FLAG_ACTIVE_LOW, &desc->flags));
>
> mutex_unlock(&data->mutex);
>
> @@ -412,7 +405,7 @@ static ssize_t base_show(struct device *dev,
> {
> const struct gpio_chip *chip = dev_get_drvdata(dev);
>
> - return sprintf(buf, "%d\n", chip->base);
> + return sysfs_emit(buf, "%d\n", chip->base);
> }
> static DEVICE_ATTR_RO(base);
>
> @@ -421,7 +414,7 @@ static ssize_t label_show(struct device *dev,
> {
> const struct gpio_chip *chip = dev_get_drvdata(dev);
>
> - return sprintf(buf, "%s\n", chip->label ? : "");
> + return sysfs_emit(buf, "%s\n", chip->label ?: "");
> }
> static DEVICE_ATTR_RO(label);
>
> @@ -430,7 +423,7 @@ static ssize_t ngpio_show(struct device *dev,
> {
> const struct gpio_chip *chip = dev_get_drvdata(dev);
>
> - return sprintf(buf, "%u\n", chip->ngpio);
> + return sysfs_emit(buf, "%u\n", chip->ngpio);
> }
> static DEVICE_ATTR_RO(ngpio);
>
> --
> 2.30.2
>

Applied, thanks!

Bartosz