* [PATCH] drivers/video: Fix -Wstringop-truncation in hdmi.c
@ 2020-10-21 12:12 Thomas Zimmermann
2020-10-21 23:06 ` Laurent Pinchart
2020-11-01 9:50 ` Sam Ravnborg
0 siblings, 2 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2020-10-21 12:12 UTC (permalink / raw)
To: b.zolnierkie, gwan-gyeong.mun, sam, daniel.vetter, bernard,
laurent.pinchart
Cc: dri-devel, linux-fbdev, Thomas Zimmermann
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)));
return 0;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] drivers/video: Fix -Wstringop-truncation in hdmi.c
2020-10-21 12:12 [PATCH] drivers/video: Fix -Wstringop-truncation in hdmi.c Thomas Zimmermann
@ 2020-10-21 23:06 ` Laurent Pinchart
2020-10-22 7:01 ` Thomas Zimmermann
2020-11-01 9:50 ` Sam Ravnborg
1 sibling, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2020-10-21 23:06 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: b.zolnierkie, gwan-gyeong.mun, sam, daniel.vetter, bernard,
dri-devel, linux-fbdev
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drivers/video: Fix -Wstringop-truncation in hdmi.c
2020-10-21 23:06 ` Laurent Pinchart
@ 2020-10-22 7:01 ` Thomas Zimmermann
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2020-10-22 7:01 UTC (permalink / raw)
To: Laurent Pinchart
Cc: b.zolnierkie, gwan-gyeong.mun, sam, daniel.vetter, bernard,
dri-devel, linux-fbdev
Hi
On 22.10.20 01:06, Laurent Pinchart wrote:
> 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
> ?
Yes, the original code this correct. I looked through include/string.h
if there's better string function, but none fits. Most of them
0-terminate the output string.
The only simple fix seems to be to set gcc's -Wno-stringop-truncation
here. I'd expect that would be an even less preferable change.
Best regards
Thomas
>
>>
>> return 0;
>> }
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/video: Fix -Wstringop-truncation in hdmi.c
2020-10-21 12:12 [PATCH] drivers/video: Fix -Wstringop-truncation in hdmi.c Thomas Zimmermann
2020-10-21 23:06 ` Laurent Pinchart
@ 2020-11-01 9:50 ` Sam Ravnborg
2020-11-01 16:43 ` Thomas Zimmermann
1 sibling, 1 reply; 5+ messages in thread
From: Sam Ravnborg @ 2020-11-01 9:50 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: b.zolnierkie, gwan-gyeong.mun, daniel.vetter, bernard,
laurent.pinchart, linux-fbdev, dri-devel
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>
I assume vendor is guaranteed to be 0-termindated.
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> ---
> 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)));
>
> return 0;
> }
> --
> 2.28.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drivers/video: Fix -Wstringop-truncation in hdmi.c
2020-11-01 9:50 ` Sam Ravnborg
@ 2020-11-01 16:43 ` Thomas Zimmermann
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2020-11-01 16:43 UTC (permalink / raw)
To: Sam Ravnborg
Cc: b.zolnierkie, gwan-gyeong.mun, daniel.vetter, bernard,
laurent.pinchart, linux-fbdev, dri-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 2726 bytes --]
Hi
Am 01.11.20 um 10:50 schrieb Sam Ravnborg:
> 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>
> I assume vendor is guaranteed to be 0-termindated.
It's a fixed-length field in the protocol. There's similar code in
radeon IIRC. So it would be nice to disable the warning in these cases.
But that's probably to much fiddling.
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Thanks.
Best regards
Thomas
>
>> ---
>> 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)));
>>
>> return 0;
>> }
>> --
>> 2.28.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.1.2: OpenPGP_0x680DC11D530B7A23.asc --]
[-- Type: application/pgp-keys, Size: 4259 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-01 16:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21 12:12 [PATCH] drivers/video: Fix -Wstringop-truncation in hdmi.c Thomas Zimmermann
2020-10-21 23:06 ` Laurent Pinchart
2020-10-22 7:01 ` Thomas Zimmermann
2020-11-01 9:50 ` Sam Ravnborg
2020-11-01 16:43 ` Thomas Zimmermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).