libgis: Use the full string length in strncmp() calls.

[infrastation]

Here is a trivial bugfix, you might want to backport it too.

[wenzeslaus]

Yep, that makes sense. Thanks for fixing it.

I'm curious how you discovered those. If it is a tool, we usually try to record that in the commit message (like "this fixes GGC warning foo-bar").

[infrastation]

It was not a tool. I was following the existing code in open.c to see why my new code does not work as expected, one strncmp() instance caught my eye and the others were a matter of grep and check.

On this note, the current build process does not enable all the warnings that GCC supports. For example, nothing pops up for unused function parameters and missing initializations. If you would like to consider a build system with more warnings enabled and support for multiple OSes and compilers, the work @guyharris and @fxlb had done in tcpdump build system is a good example.

[wenzeslaus]

It was not a tool. I was following the existing code in open.c to see why my new code does not work as expected, one strncmp() instance caught my eye and the others were a matter of grep and check.

Thanks for looking also beyond open.c!

On this note, the current build process does not enable all the warnings that GCC supports. For example, nothing pops up for unused function parameters and missing initializations.

Right. Ideally, we would get most of those, e.g., whatever is default on Debian. Any changes in that regard are welcome. I set up standard check for C/C++ in GitHub Actions, but it is for GNU C, not ISO C, and it is not strict. We were able to significantly reduce issues in Python code recently, but more work is needed there too. Perhaps we can discuss this during the sprint.

If you would like to consider a build system with more warnings enabled and support for multiple OSes and compilers, the work @guyharris and @fxlb had done in tcpdump build system is a good example.

It seems they are now using CMake. We have an open PR for CMake, but neither the OP nor myself touched it for some time. Also something to discuss more.

[infrastation]

Thank you. CMake is a relatively recent addition there, but I meant the autoconf way. If you remind me during the meeting, I can give a brief overview.

[guyharris]

If you would like to consider a build system with more warnings enabled and support for multiple OSes and compilers, the work @guyharris and @fxlb had done in tcpdump build system is a good example.

It seems they are now using CMake. We have an open PR for CMake, but neither the OP nor myself touched it for some time. Also something to discuss more.

libpcap and tcpdump now support both autotools and CMake for building. The primary reason to add CMake support was to add support for building on Windows in a fashion that wasn't Windows-only and could be maintained by people on UN*X systems.

[wenzeslaus]

If you would like to consider a build system with more warnings enabled and support for multiple OSes and compilers, the work @guyharris and @fxlb had done in tcpdump build system is a good example.

It seems they are now using CMake. We have an open PR for CMake, but neither the OP nor myself touched it for some time. Also something to discuss more.

libpcap and tcpdump now support both autotools and CMake for building. The primary reason to add CMake support was to add support for building on Windows in a fashion that wasn't Windows-only and could be maintained by people on UN*X systems.

Good to know. Having CMake alongside Autotools was what we had in mind too. CMake seems to make a lot of sense. We actually do have a Windows build with autotools, but not with MSVC.