| Message ID | 20201021121241.17623-1-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | drivers/video: Fix -Wstringop-truncation in hdmi.c | expand |
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; > }
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; >> } >
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
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
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; }
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(-)