linux-kernel - Re: [PATCH] drm/bridge: it6505: replace scnprintf with sysfs_emit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2025063006-recopy-playmaker-562d@gregkh>
Date: Mon, 30 Jun 2025 07:09:50 +0200
From: Greg KH <greg@...ah.com>
To: Marcelo Moreira <marcelomoreira1905@...il.com>
Cc: Andrzej Hajda <andrzej.hajda@...el.com>, skhan@...uxfoundation.org,
	linux-kernel-mentees@...ts.linuxfoundation.org,
	~lkcamp/patches@...ts.sr.ht,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Robert Foss <rfoss@...nel.org>,
	Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
	Jonas Karlman <jonas@...boo.se>,
	Jernej Skrabec <jernej.skrabec@...il.com>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/bridge: it6505: replace scnprintf with sysfs_emit_at
 in debugfs show

On Sun, Jun 29, 2025 at 08:35:09PM -0300, Marcelo Moreira wrote:
> Update the receive_timing_debugfs_show() function to utilize
> sysfs_emit_at() for formatting output to the debugfs buffer.
> This change adheres to the recommendation outlined
> in Documentation/filesystems/sysfs.rst.
> 
> This modification aligns with current sysfs guidelines.

But this isn't a sysfs file, it's a debugfs file, so why are you calling
sysfs_emit_at()?

> 
> Signed-off-by: Marcelo Moreira <marcelomoreira1905@...il.com>
> ---
>  drivers/gpu/drm/bridge/ite-it6505.c | 46 ++++++++++++++---------------
>  1 file changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 1383d1e21afe..98bea08a14e4 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -3427,37 +3427,35 @@ static ssize_t receive_timing_debugfs_show(struct file *file, char __user *buf,
>  	struct it6505 *it6505 = file->private_data;
>  	struct drm_display_mode *vid;
>  	u8 read_buf[READ_BUFFER_SIZE];
> -	u8 *str = read_buf, *end = read_buf + READ_BUFFER_SIZE;
> -	ssize_t ret, count;
> +	ssize_t ret;
> +	ssize_t count = 0;
>  
>  	if (!it6505)
>  		return -ENODEV;
>  
>  	it6505_calc_video_info(it6505);
>  	vid = &it6505->video_info;
> -	str += scnprintf(str, end - str, "---video timing---\n");
> -	str += scnprintf(str, end - str, "PCLK:%d.%03dMHz\n",
> -			 vid->clock / 1000, vid->clock % 1000);
> -	str += scnprintf(str, end - str, "HTotal:%d\n", vid->htotal);
> -	str += scnprintf(str, end - str, "HActive:%d\n", vid->hdisplay);
> -	str += scnprintf(str, end - str, "HFrontPorch:%d\n",
> -			 vid->hsync_start - vid->hdisplay);
> -	str += scnprintf(str, end - str, "HSyncWidth:%d\n",
> -			 vid->hsync_end - vid->hsync_start);
> -	str += scnprintf(str, end - str, "HBackPorch:%d\n",
> -			 vid->htotal - vid->hsync_end);
> -	str += scnprintf(str, end - str, "VTotal:%d\n", vid->vtotal);
> -	str += scnprintf(str, end - str, "VActive:%d\n", vid->vdisplay);
> -	str += scnprintf(str, end - str, "VFrontPorch:%d\n",
> -			 vid->vsync_start - vid->vdisplay);
> -	str += scnprintf(str, end - str, "VSyncWidth:%d\n",
> -			 vid->vsync_end - vid->vsync_start);
> -	str += scnprintf(str, end - str, "VBackPorch:%d\n",
> -			 vid->vtotal - vid->vsync_end);
> -
> -	count = str - read_buf;
> +	count += sysfs_emit_at(read_buf, count, "---video timing---\n");
> +	count += sysfs_emit_at(read_buf, count, "PCLK:%d.%03dMHz\n",
> +			vid->clock / 1000, vid->clock % 1000);
> +	count += sysfs_emit_at(read_buf, count, "HTotal:%d\n", vid->htotal);
> +	count += sysfs_emit_at(read_buf, count, "HActive:%d\n", vid->hdisplay);
> +	count += sysfs_emit_at(read_buf, count, "HFrontPorch:%d\n",
> +			vid->hsync_start - vid->hdisplay);
> +	count += sysfs_emit_at(read_buf, count, "HSyncWidth:%d\n",
> +			vid->hsync_end - vid->hsync_start);
> +	count += sysfs_emit_at(read_buf, count, "HBackPorch:%d\n",
> +			vid->htotal - vid->hsync_end);
> +	count += sysfs_emit_at(read_buf, count, "VTotal:%d\n", vid->vtotal);
> +	count += sysfs_emit_at(read_buf, count, "VActive:%d\n", vid->vdisplay);
> +	count += sysfs_emit_at(read_buf, count, "VFrontPorch:%d\n",
> +			vid->vsync_start - vid->vdisplay);
> +	count += sysfs_emit_at(read_buf, count, "VSyncWidth:%d\n",
> +			vid->vsync_end - vid->vsync_start);
> +	count += sysfs_emit_at(read_buf, count, "VBackPorch:%d\n",
> +			vid->vtotal - vid->vsync_end);
> +	
>  	ret = simple_read_from_buffer(buf, len, ppos, read_buf, count);
> -

Shouldn't this all be using seq_print() instead?

Again, don't use sysfs_emit*() functions for non-sysfs files, as you do
NOT know the size of the buffer here (hint, it's not the same).

And, your patch added trailing whitespace, did you forget to run it
through checkpatch.pl before sending it?

thanks,

greg k-h