kosma / minmea

a lightweight GPS NMEA 0183 parser library in pure C
Do What The F*ck You Want To Public License
756 stars 246 forks source link

Add $--VTG sentences. #11

Closed lvitya closed 8 years ago

lvitya commented 9 years ago

Oops! Build with clang failed. I will try to figure it out.

lvitya commented 9 years ago

Can someone suggest me how to repeat this on my local machine? When I run "make CC=clang" it says me "+++ All good."

swistakm commented 9 years ago

@lvitya I have reproduced this on older version of clang:

vagrant@vagrant-debian-wheezy:/vagrant/minmea$ uname -a
Linux vagrant-debian-wheezy 3.2.0-4-amd64 #1 SMP Debian 3.2.60-1+deb7u3 x86_64 GNU/Linux

vagrant@vagrant-debian-wheezy:/vagrant/minmea$ clang --version
Debian clang version 3.0-6.2 (tags/RELEASE_30/final) (based on LLVM 3.0)
Target: x86_64-pc-linux-gnu
Thread model: posix

vagrant@vagrant-debian-wheezy:/vagrant/minmea$ make CC=clang tests
clang -g -Wall -Wextra -Werror -std=c99 -D_POSIX_C_SOURCE=199309L -D_BSD_SOURCE -D_DARWIN_C_SOURCE   -c -o tests.o tests.c
clang -g -Wall -Wextra -Werror -std=c99 -D_POSIX_C_SOURCE=199309L -D_BSD_SOURCE -D_DARWIN_C_SOURCE   -c -o minmea.o minmea.c
clang   tests.o minmea.o  -lcheck -lm -o tests

vagrant@vagrant-debian-wheezy:/vagrant/minmea$ ./tests 
Running suite(s): minmea
96%: Checks: 32, Failures: 1, Errors: 0
tests.c:588:F:minmea_parse:test_minmea_parse_gll1:0: Assertion '!memcmp(&frame, &expected, sizeof(frame))' failed

Other environment -> tests pass:

[vtg]swistakm@spock:~/dev/minmea$ uname -a
Darwin spock 14.0.0 Darwin Kernel Version 14.0.0: Fri Sep 19 00:26:44 PDT 2014; root:xnu-2782.1.97~2/RELEASE_X86_64 x86_64

[vtg]swistakm@spock:~/dev/minmea$ cc --version
Apple LLVM version 6.0 (clang-600.0.56) (based on LLVM 3.5svn)
Target: x86_64-apple-darwin14.0.0
Thread model: posix

[vtg]swistakm@spock:~/dev/minmea$ make tests
cc -g -Wall -Wextra -Werror -std=c99 -D_POSIX_C_SOURCE=199309L -D_BSD_SOURCE -D_DARWIN_C_SOURCE   -c -o tests.o tests.c
cc -g -Wall -Wextra -Werror -std=c99 -D_POSIX_C_SOURCE=199309L -D_BSD_SOURCE -D_DARWIN_C_SOURCE   -c -o minmea.o minmea.c
cc   tests.o minmea.o  -lcheck -o tests

[vtg]swistakm@spock:~/dev/minmea$ ./tests 
Running suite(s): minmea
100%: Checks: 32, Failures: 0, Errors: 0
lvitya commented 9 years ago

I use cygwin and it doesn't have this version of clang. It seems to be the tricky task.

kkszysiu commented 9 years ago

Maybe run any Linux distro on VirtualBox, it would be much easier to work with than Cygwin. Also this might be Cygwin fault itself.

kosma commented 9 years ago

Hi @lvitya, thanks for the patches. The tests situation is a fuckup on my part; I assumed compilers would zero out the padding but they don't (and it's a valid behaviour according to C99). I'll have a good look and fix it across the entire library so that it doesn't happen again. Same with clang failures. It's might take a few days though; I lost write access to this repo in a knitting accident and I need to contact some humans to get it back.

lvitya commented 9 years ago

@kosma I faced with padding problem when run tests of cloned repo on my machine. However, as you can see from my commits, I fix it in this particular "test_minmea_parse_gll1". So it is not clear that this test failure related to padding problem.

kosma commented 9 years ago

@lvitya The reason for padding errors is because C99 doesn't require padding to be zero-initialized. The following fixes work, but invoke undefined behavior:

I think we can memset to zero, then fill values, then memcmp. Of course, comparing fields one-by-one would be safest, but I'd rather avoid that kind of stuff in unit tests.

Still waiting for the repo write access; I'll prepare fixes for the padding issue in the meantime.

maximevince commented 8 years ago

Ok, I just noticed I have the same kind of struct comparison / non-initialized padding problem on my machine. :) I opened #14 for that, before I read this comment.

lvitya commented 8 years ago

@maximevince Don't know why, but my pull request already waiting for a year to be accepted. So maybe it's worth for you to make your own fork of this project.