[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aa43c1f8-05bc-4edd-b7ba-474953f28f5c@stanley.mountain>
Date: Fri, 9 Aug 2024 17:42:32 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: Helge Deller <deller@....de>, linux-kernel@...r.kernel.org,
kernel-janitors@...r.kernel.org, linux-omap@...r.kernel.org,
linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] fbdev: omapfb: panel-sony-acx565akm: Simplify
show_cabc_available_modes()
On Thu, Aug 08, 2024 at 11:46:11AM +0200, Christophe JAILLET wrote:
> Use sysfs_emit_at() instead of snprintf() + custom logic.
> Using sysfs_emit_at() is much more simple.
>
> Also, sysfs_emit() is already used in this function, so using
> sysfs_emit_at() is more consistent.
>
> Also simplify the logic:
> - always add a space after an entry
> - change the last space into a '\n'
>
> Finally it is easy to see that, given the size of cabc_modes, PAGE_SIZE
> can not be reached.
> So better keep everything simple (and correct).
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> ---
> .../omap2/omapfb/displays/panel-sony-acx565akm.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
> index 71d2e015960c..fc975615d5c9 100644
> --- a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
> +++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
> @@ -466,19 +466,20 @@ static ssize_t show_cabc_available_modes(struct device *dev,
> char *buf)
> {
> struct panel_drv_data *ddata = dev_get_drvdata(dev);
> - int len;
> + int len = 0;
> int i;
>
> if (!ddata->has_cabc)
> return sysfs_emit(buf, "%s\n", cabc_modes[0]);
>
> - for (i = 0, len = 0;
> - len < PAGE_SIZE && i < ARRAY_SIZE(cabc_modes); i++)
> - len += snprintf(&buf[len], PAGE_SIZE - len, "%s%s%s",
> - i ? " " : "", cabc_modes[i],
> - i == ARRAY_SIZE(cabc_modes) - 1 ? "\n" : "");
> + for (i = 0; i < ARRAY_SIZE(cabc_modes); i++)
> + len += sysfs_emit_at(buf, len, "%s ", cabc_modes[i]);
> +
> + /* Remove the trailing space */
> + if (len)
> + buf[len - 1] = '\n';
I'm uncomfortable with this line. It assumes we don't overflow PAGE_SIZE where
the original code was careful about checking. Probably easiest to do what the
original code did and say:
for (i = 0; i < ARRAY_SIZE(cabc_modes); i++)
len += sysfs_emit_at(buf, len, "%s%s", cabc_modes[i],
i == ARRAY_SIZE(cabc_modes) - 1 ? "\n" : "");
regards,
dan carpenter
Powered by blists - more mailing lists