Re: [PATCH] drivers/video: Fix -Wstringop-truncation in hdmi.c

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: b.zolnierkie@samsung.com, gwan-gyeong.mun@intel.com,
	sam@ravnborg.org, daniel.vetter@ffwll.ch, bernard@vivo.com,
	dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org
Subject: Re: [PATCH] drivers/video: Fix -Wstringop-truncation in hdmi.c
Date: Thu, 22 Oct 2020 02:06:54 +0300	[thread overview]
Message-ID: <20201021230654.GS3942@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20201021121241.17623-1-tzimmermann@suse.de>

Hi Thomas,

Thank you for the patch.

On Wed, Oct 21, 2020 at 02:12:41PM +0200, Thomas Zimmermann wrote:
> Trying to copy into the string fields with strncpy() gives a warning from
> gcc. Both fields are part of a packed HDMI header and do not require a
> terminating \0 character.
> 
> ../drivers/video/hdmi.c: In function 'hdmi_spd_infoframe_init':
> ../drivers/video/hdmi.c:230:2: warning: 'strncpy' specified bound 8 equals destination size [-Wstringop-truncation]
>   230 |  strncpy(frame->vendor, vendor, sizeof(frame->vendor));
>       |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/video/hdmi.c:231:2: warning: 'strncpy' specified bound 16 equals destination size [-Wstringop-truncation]
>   231 |  strncpy(frame->product, product, sizeof(frame->product));
>       |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Just use memcpy() instead.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/video/hdmi.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index b7a1d6fae90d..1e4cb63d0d11 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -221,14 +221,18 @@ EXPORT_SYMBOL(hdmi_avi_infoframe_pack);
>  int hdmi_spd_infoframe_init(struct hdmi_spd_infoframe *frame,
>  			    const char *vendor, const char *product)
>  {
> +	size_t len;
> +
>  	memset(frame, 0, sizeof(*frame));
>  
>  	frame->type = HDMI_INFOFRAME_TYPE_SPD;
>  	frame->version = 1;
>  	frame->length = HDMI_SPD_INFOFRAME_SIZE;
>  
> -	strncpy(frame->vendor, vendor, sizeof(frame->vendor));
> -	strncpy(frame->product, product, sizeof(frame->product));
> +	len = strlen(vendor);
> +	memcpy(frame->vendor, vendor, min(len, sizeof(frame->vendor)));
> +	len = strlen(product);
> +	memcpy(frame->product, product, min(len, sizeof(frame->product)));

As this seems to be a legitimate use of strncpy(), isn't there a way to
silence the warning without requiring this additional runtime complexity
?

>  
>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart