Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Building with gcc8 fails #2

Open
eli-schwartz opened this issue Jul 8, 2018 · 3 comments
Open

Building with gcc8 fails #2

eli-schwartz opened this issue Jul 8, 2018 · 3 comments

Comments

@eli-schwartz
Copy link

Scanning dependencies of target forensic1394
[ 16%] Building C object CMakeFiles/forensic1394.dir/src/common.c.o
[ 33%] Building C object CMakeFiles/forensic1394.dir/src/linux/juju.c.o
In function ‘alloc_dev.isra.1’,
    inlined from ‘platform_update_device_list’ at /build/libforensic1394/src/libforensic1394-0.2/src/linux/juju.c:275:41:
/build/libforensic1394/src/libforensic1394-0.2/src/linux/juju.c:367:5: error: ‘strncpy’ specified bound 64 equals destination size [-Werror=stringop-truncation]
     strncpy(dev->pdev->path, devpath, sizeof(dev->pdev->path));
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [CMakeFiles/forensic1394.dir/build.make:76: CMakeFiles/forensic1394.dir/src/linux/juju.c.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:68: CMakeFiles/forensic1394.dir/all] Error 2
make: *** [Makefile:152: all] Error 2

This happens for two reasons:

  • The code has not been touched in 8 years, and since then gcc 8 added new compiler warnings.
  • The CMakeLists.txt enables -Werror, with the comment "The more warnings the better" (which is incorrect, -Werror does not add new warnings. It just prevents people from compiling stable releases anyway).

-Werror should not be used in release tarballs. Accepted practice is to enable -Werror via environment CFLAGS, for people who develop software to add those environment CFLAGS, and to enable those environment CFLAGS in e.g. Travis CI configurations.

Using -Werror in the release code is especially bad for codebases which are not actively maintained since they eventually become impossible to build, not because the code itself is fundamentally flawed, but because compilers have added "best practices" warnings years later.

Also, this warning should probably be fixed. :)

@FreddieWitherden
Copy link
Owner

Both of these issues should now be fixed in the current master branch.

Note that compared with 0.2 the master branch has a custom CSR ROM parser. Although I believe it to be relatively bug-free, and tested it extensively back in 2011, it nevertheless could do with more testing.

@fabaff
Copy link

fabaff commented May 9, 2019

Can you release create a new release with the changes?

The package for Fedora doesn0t build anymore.

Thanks

@FreddieWitherden
Copy link
Owner

Would you be in a position to provide a second confirmation that the current git head builds and works as expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants