search

jacketizer / libnmea

Lightweight C library for parsing NMEA 0183 sentences
MIT License
264 stars 93 forks source link

Enable -Werror, fix related warnings, fixes for PARSER_PATH #47

Closed igrr closed 3 years ago

igrr commented 3 years ago

  1. This PR enables -Werror flag when building the library, examples, and tests. -Wall which was missing in CMake is also added. The warnings found after enabling these flags are fixed:

    • parser_path = parser_path; assignment in parser.c (-Wself-assign)
    • NULL == parsers[i].parser.type_word in parser_static.c (-Wtautological-pointer-compare, since type_word array address can never be NULL)
    • test_parse_invalid in test_lib.c never invoked

    • PARSER_PATH redefinition in parser.c — it was also defined on the command line. Once this was fixed, it turned out that:

      • PARSER_PATH definition on the command line (in Make) was missing quotes around the string constant.
      • PARSER_PATH was not defined in CMakeLists.txt the same way it was defined in Make.

      Both issues were also fixed.

  2. A note about NMEA_PARSER_PATH variable is added to readme file (#42).

Note that while -Wall -Werror is now enabled in CMake for all the files (library, example, tests), this is not the case in Make. Makefile does not use CFLAGS consistently for all the CC invocations, so some sources are compiled without -Wall -Werror.

I started cleaning this up, but then ran into the problem that I can't compile with Make on macOS at all, due to the usage of linker flags such as --no-as-needed, -soname, --exclude-libs=ALL, none of which is supported on macOS. Curiously, these flags are not present in CMakeLists.txt.

I have opted not to add these flags to CMakeLists.txt, to keep at least that useable on macOS :). For the time being I have dropped the attempt to fix CFLAGS inconsistencies in the Makefile.

igrr commented 3 years ago

@jacketizer If you are going to be reviewing this, please consider merging https://github.com/jacketizer/libnmea/pull/40 first as it has larger set of changes. If some conflicts arise i can rebase this one after #40 is merged.