After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 795756 - Compilation fails on GCC 8.0.1
Compilation fails on GCC 8.0.1
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal minor
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-05-02 15:57 UTC by U. Artie Eoff
Modified: 2018-05-07 15:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gst: Use memcpy() instead of strncpy() where appropriate (1.74 KB, patch)
2018-05-04 07:31 UTC, Edward Hervey
committed Details | Review

Description U. Artie Eoff 2018-05-02 15:57:05 UTC
gstreamer (master) heads/master-0-g98200ddd8dfd

With GCC 8.0.1 (default on recent Fedora 28 release), gstreamer fails to compile:

gstregistrybinary.c: In function 'gst_registry_binary_initialize_magic':
gstregistrybinary.c:339:8: error: 'strncpy' output truncated before terminating nul copying 4 bytes from a string of the same length [-Werror=stringop-truncation]
   if (!strncpy (m->magic, GST_MAGIC_BINARY_REGISTRY_STR,
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           GST_MAGIC_BINARY_REGISTRY_LEN)
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[4]: *** [Makefile:1617: libgstreamer_1.0_la-gstregistrybinary.lo] Error 1
Comment 1 Sebastian Dröge (slomo) 2018-05-02 19:30:33 UTC
False positive, but this should really use memcpy instead. Want to provide a patch?
Comment 2 Edward Hervey 2018-05-04 07:31:32 UTC
Created attachment 371655 [details] [review]
gst: Use memcpy() instead of strncpy() where appropriate

strncpy() is assumed to be for strings so the compiler assumes that
it will need an extra byte for the string-terminaning NULL.

For cases where we know it's actually "binary" data, just copy it
with memcpy.
Comment 3 Sebastian Dröge (slomo) 2018-05-04 08:15:44 UTC
Review of attachment 371655 [details] [review]:

Do we change the binary format of the registry now potentially, because it previously had a 0 and not anymore?

::: gst/gstregistrybinary.c
@@ +340,3 @@
           GST_MAGIC_BINARY_REGISTRY_LEN)
       || !strncpy (m->version, GST_MAGIC_BINARY_VERSION_STR,
           GST_MAGIC_BINARY_VERSION_LEN)) {

There's another strncpy() here
Comment 4 Edward Hervey 2018-05-05 08:23:11 UTC
Review of attachment 371655 [details] [review]:

::: gst/gstregistrybinary.c
@@ +340,3 @@
           GST_MAGIC_BINARY_REGISTRY_LEN)
       || !strncpy (m->version, GST_MAGIC_BINARY_VERSION_STR,
           GST_MAGIC_BINARY_VERSION_LEN)) {

That length is 64 :)
Comment 5 Edward Hervey 2018-05-05 08:24:16 UTC
> Do we change the binary format of the registry now potentially, because it
> previously had a 0 and not anymore?

  It didn't. It was a 4 byte "string" put into at maximum 4 bytes. Nothing changes, it's just the warning that goes away.
Comment 6 Edward Hervey 2018-05-07 15:12:41 UTC
Attachment 371655 [details] pushed as 80dfb7b - gst: Use memcpy() instead of strncpy() where appropriate