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

Make snoopy compile on 64-bit Arch Linux with GCC 7.1.1 #122

Merged
merged 1 commit into from Apr 10, 2018

Conversation

xyproto
Copy link
Contributor

@xyproto xyproto commented Jul 27, 2017

Checklists for Pull requests

About pull request itself:

Code quality:
(not applicable for non-code fixes of course)

  • My submission is passing test suite run (./configure --enable-everything && make tests) - test suite reports zero unexpected failures. One of the tests fails, but I can not test if it already failed before this commit, since it does not compile here.
  • My submission is passing Valgrind check (./configure --enable-everything --enable-dev-tools && make valgrind) - Valgrind reports "ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)".

Commits:

  • My commits are logical, easily readable, with concise comments.
  • My commits follow the KISS principle: do one thing, and do it well.

Licensing:

  • I am the author of submission or have been authorized by submission copyright holder to issue this pull request.

Branching:

  • My submission is based on master branch.
  • My submission is compatible with latest master branch updates (no conflicts, I did a rebase if it was necessary).
  • The name of the branch I want to merge upstream is not 'master' (except for only the most trivial fixes, like typos and such).
  • My branch name is feature/my-shiny-new-snoopy-feature-title (for new features).
  • My branch name is bugfix/my-totally-non-hackish-workaround (for bugfixes).
  • My branch name is doc/what-i-did-to-documentation (for documentation updates).

Continuous integration:

  • Once I will submit this pull request, I will wait for Travis-CI report (normally a couple of minutes) and fix any issues I might have introduced.

Pull request description

This commit makes snoopy compile, but someone must confirm that the functionality is the same.

The error messages I get before this commit are:

  CC       cmdline.lo
cmdline.c: In function ‘snoopy_datasource_cmdline’:
cmdline.c:71:79: error: comparison between pointer and zero character constant [-Werror=pointer-compare]
     for (cmdLineArgCount=0 ; *(snoopy_inputdatastorage->argv+cmdLineArgCount) != '\0' ; cmdLineArgCount++);
                                                                               ^~
cmdline.c:71:30: note: did you mean to dereference the pointer?
     for (cmdLineArgCount=0 ; *(snoopy_inputdatastorage->argv+cmdLineArgCount) != '\0' ; cmdLineArgCount++);
                              ^
cc1: all warnings being treated as errors
make[2]: *** [Makefile:788: cmdline.lo] Error 1

After this commit, snoopy at least compiles.

@bostjan
Copy link
Member

bostjan commented Jul 28, 2017

Hi @xyproto, thanks for the contribution.

Did you run the test suite locally on arch, did it work?

Arch is rolling-release distro, right? Can you give any point of reference if I would want to replicate the behaviour locally?

@xyproto
Copy link
Contributor Author

xyproto commented Jul 28, 2017

@bostjan Yes, I ran the test suit locally and it did not work, but I don't know if it did not work also before my patch or not. I commented this in the form above.

I guess the easiest way to test this is to run Arch Linux in a virtual machine or as a docker image. If you don't want any manual setup, something like installing Antergos in VirtualBox should work as well.

@lukas227
Copy link
Contributor

lukas227 commented Aug 5, 2017

We have this problem in Debian as well (see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=853664) now that gcc-7 is default. I am thinking of applying the same patch (except that I'd rather use NULL instead of 0). With the patch, the test suite run after the build passes (except for 4 specific tests which we needed to disable in Debian due to the special environment in which the build is run, see https://sources.debian.net/src/snoopy/2.4.6-1/debian/patches/03_disable-tests.patch/).

@bostjan
Copy link
Member

bostjan commented Feb 12, 2018

Tests were working the last time I checked. Which ones are failing?

@bostjan
Copy link
Member

bostjan commented Apr 10, 2018

Looks good. Thanks!

About config.h.in change: I guess I'll start maintaining Snoopy on Ubuntu 18.04 now :)

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

Successfully merging this pull request may close these issues.

Build error on Amazon Linux 2 Errors building on el8 Snoopy Fails to build from source with gcc-7
3 participants