Bump the C++ version from C++11 to C++17

[MirkoDziadzka]

This will allow the usage of more modern features in the future.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[gberkes]

Hi Mirko!

Before last xmas I built ModSec with these flags:
CXXFLAGS=-std=c++20 -Wall -Wextra -Wconversion -Wshadow -Wpedantic -fsanitize=undefined,address
LDFLAGS=-fsanitize=undefined,address
It produced a huge amount of warnings. I read about secure coding and found, that one of the best practices is to fully utilize the language and the compiler provided features to enhance code clarity and security before using 3rd party static and dynamic analyzers.

I kindly ask what is your opinion not to stop at C++17 but bump it to C++20, and introduce more compiler and linker flags to promote higher level code quality?

My goal is not to use new language features as much as possible at all cost, but let the language and the compiler
help us write nicer, cleaner and more secure code.

References:
https://wiki.sei.cmu.edu/confluence/display/seccode/Top+10+Secure+Coding+Practices
https://learning.oreilly.com/library/view/c-coding-standards/0321113586/ch02.html
https://www.sonarsource.com/blog/modernizing-your-code-with-cpp20/

[airween]

Hi @MirkoDziadzka, thanks for sent this patch.

I took a quick review on QA logs and compared them with a previous build results. Here are some details just FYI:

• on MacOSX the autoconf has changed, but that produces some not relevant outputs

35c35
< ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: Warning: autoconf 2.71 is already installed and up-to-date.
---
> ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: Warning: autoconf 2.72 is already installed and up-to-date.
38,40c38,40
< ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:106: warning: The macro `AC_TRY_COMPILE' is obsolete.
< ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:129: warning: The macro `AC_TRY_LINK' is obsolete.
< ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:140: warning: The macro `AC_HEADER_STDC' is obsolete.
---
> ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:106: warning: The macro 'AC_TRY_COMPILE' is obsolete.
> ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:129: warning: The macro 'AC_TRY_LINK' is obsolete.
> ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:140: warning: The macro 'AC_HEADER_STDC' is obsolete.
42,44c42,44
< ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:106: warning: The macro `AC_TRY_COMPILE' is obsolete.
< ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:129: warning: The macro `AC_TRY_LINK' is obsolete.
< ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:140: warning: The macro `AC_HEADER_STDC' is obsolete.
---
> ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:106: warning: The macro 'AC_TRY_COMPILE' is obsolete.
> ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:129: warning: The macro 'AC_TRY_LINK' is obsolete.
> ./7_build-macos (macos-12, clang, wo maxmind, --without-maxmin.txt: configure.ac:140: warning: The macro 'AC_HEADER_STDC' is obsolete.

• and there are some Yacc warnings:

50,51c50,52
< ./build-linux (ubuntu-22.04, x32, clang, with parser generation, --enable-parser-generation)/7_make.txt: /home/runner/work/ModSecurity/ModSecurity/src/parser/seclang-scanner.ll:924: warning, rule cannot be matched
< ./build-linux (ubuntu-22.04, x32, clang, with parser generation, --enable-parser-generation)/7_make.txt: /home/runner/work/ModSecurity/ModSecurity/src/parser/seclang-scanner.ll:1077: warning, rule cannot be matched
---
> ./build-linux (ubuntu-22.04, x32, clang, with parser generation, --enable-parser-generation)/7_make.txt: /home/runner/work/ModSecurity/ModSecurity/src/parser/seclang-parser.yy:4.1-42: warning: deprecated directive: ‘%define parser_class_name {seclang_parser}’, use ‘%define api.parser.class {seclang_parser}’ [-Wdeprecated]
> ./build-linux (ubuntu-22.04, x32, clang, with parser generation, --enable-parser-generation)/7_make.txt: /home/runner/work/ModSecurity/ModSecurity/src/parser/seclang-parser.yy: warning: 1272 shift/reduce conflicts [-Wconflicts-sr]
> ./build-linux (ubuntu-22.04, x32, clang, with parser generation, --enable-parser-generation)/7_make.txt: /home/runner/work/ModSecurity/ModSecurity/src/parser/seclang-parser.yy: warning: fix-its can be applied.  Rerun with option '--update'. [-Wother]

Perhaps this one is because of the changed of the flags, but I guess those are also not relevant. So it looks like there are no limiting factors to switch to C++17. Let us wait for some other opinions.

[mirkodziadzka-avi]

I kindly ask what is your opinion not to stop at C++17 but bump it to C++20, and introduce more compiler and linker flags to promote higher level code quality?

I'm all in to add all compiler warnings and static analyzer flags we can. But from experience, this is a slow process because you have to fix a lot on the way. But yes, it is the right way.

Regarding compiler version: We can bump it to C++20. 17 is a compromise where a lot of modern C++ is already there and what should be supported on all modern systems. 20 may be too young for some build environments. I don't really know. YMMV

This PR is more or less: I want to fix another problem, for performance reasons, I would prefer to have std::string_view and not only std::string and therefore I did open this PR.

Maybe we need to distinguish between the production build compiler version which dictates the language feature versions we can use and the dev compiler build which can used the newest compiler and the best static analyzer tools.

[gberkes]

Maybe we need to distinguish between the production build compiler version which dictates the language feature versions we can use and the dev compiler build which can used the newest compiler and the best static analyzer tools.

That's a great idea. I'll work that way. Thank you very much.

[zimmerle]

Building on our previous discussions in 2020 regarding the adoption of std::string_view, as captured in the GitHub pull request here, it's worth noting that there might already exist a branch where this development has been undertaken. At that time, we encountered compiler-related issues on several widely-used distributions. Additionally, we explored the option of integrating standalone implementations of std::string_view, such as the one available at martinmoene/string-view-lite, to mitigate these challenges. We dropped as won't be wide spread, some vendors adopted as they had infra for it. I voluntear to review anything on that segment. The std::string_view is particullary interesting with the utilization of an API expansion that would allow strings be constructed with buffer + len. As oppose to only buffer. That would avoid copies on the connectors as well.

@majordaw @airween you can count on me to review the std::string_view implemetation.

[airween]

Looks good to me.

If there is no other remark from anyone, I'm going to merge this PR in next few days.