I don't know whether all LTS Linux distributions containing/supporting ModSecurity have GCC capable of compiling with -std=C++20. I guess users of ModSecurity have little interest in which standard we are using.

As a user of a security product, I would be much more interested in the quality of the codebase the product is built from. First of all, IMHO, we should not only focus on what SonarCloud reports but on what the G++ compiler would report with the appropriate compiler and linker flags used throughout development and testing. Currently, we are using almost none of what G++ could provide regarding the quality of our code. Instead, we are relying on Cppcheck, with many suppressions, some without explanations as to why they are false positives.

So, IMHO, it would be a better idea to first use something like: g++ -std=c++17 -g -Wall -Wextra -Wconversion -Wshadow -Wpedantic -fsanitize=undefined,address during development and testing, and refactor the code to build warning-free. Besides this, making an effort to eliminate or explain the Cppcheck suppressions would be beneficial.

IMHO, we developers and the end users of ModSecurity will benefit more from these refactorings than from using newer language features. For example, until the next release, one of the goals could be to achieve a warning-free build, extend test coverage, and plan which parts of the code would be good to refactor using the new standard's constructs. After that, there's nothing against bumping to C++20, except the supported OSes' GCC standard coverage.

Reference:
https://wiki.sei.cmu.edu/confluence/display/c/MSC00-C.+Compile+cleanly+at+high+warning+levels

I don't disagree. As I stated in the description to this PR, the goal is not to have this integrated now, but rather to document how to update the compiler version in the future and also to provide visibility on whether the codebase compiles with C++20 (as I mentioned, it didn't with C++23 due to a couple of minor issues -that apply in C++17 as well-):

The goal of this PR is not necessarily to update this configuration at this time, but rather to document that the library builds correctly at this stage using std C++20 on all platforms.

I wouldn't minimize the current state of the codebase, though, as compiling with no warnings and cppcheck (with few suppressions) is significant. Of course, raising the bar is always good and could be a focus for improvement. Earlier this year I proposed using the latest version ofcppcheck and have a branch with that, which I've not submitted because it takes significantly longer to analyze the codebase and there's a trade-off to consider there.

With regards to sonarcloud, it's a nice addition as well to the analysis of the codebase. I think it's helpful to focus on issues related to security (and performance) though. In my experience, it's reporting many suggestions on leveraging modern C++ features such asstructured binding or replacingNULL withnullptr, which are nice and can be adopted gradually as the codebase evolves with actual functional changes and fixes.

Hi@eduar-hte,

I don't disagree. As I stated in the description to this PR, the goal is not to have this integrated now, but rather to document how to update the compiler version in the future and also to provide visibility on whether the codebase compiles with C++20 (as I mentioned, it didn't with C++23 due to a couple of minor issues -that apply in C++17 as well-):

then may be you should add a small block into the README.

With this PR you requested to change the c++ version from 17 to 20.

I downloaded your branch into a Debian 11 (which is still a supported system) and I got:

...checking pkg-config is at least version 0.9.0... yeschecking whether g++ supports C++20 features with -std=c++20... nochecking whether g++ supports C++20 features with +std=c++20... nochecking whether g++ supports C++20 features with -h std=c++20... nochecking whether g++ supports C++20 features with -std:c++20... noconfigure: error: *** A compiler with support for C++20 language features is required.

So I'm not able to build the library.

Note: Debian 11 uses gcc 10.2, Ubuntu Focal (20.04 - this is an LTS system) uses gcc 9.3.

The goal of this PR is not necessarily to update this configuration at this time, but rather to document that the library builds correctly at this stage using std C++20 on all platforms.

But this PR definitely updates the configuration, doesn't it?

But this PR definitely updates the configuration, doesn't it?

Yes, but I'm not proposing integrating this PR right now, but at some point in the future.

I don't think there's any urgency to move to C++20. Moreover, my main concern would be upgrading to that version and start adopting theranges library suggestions bysonarcloud and having a negative impact on performance, just because the tool said so.

I've been building the library myself using the default configuration, but also with newer versions of the standard as well, in order to detect any potential and unnecessary roadblock to upgrading in the future. This PR can easily be rebased as well to keep track of this until the moment is right to integrate it.

But this PR definitely updates the configuration, doesn't it?

Yes, but I'm not proposing integrating this PR right now, but at some point in the future.

Ahh, right - then please feel free to add labels on the right side of the page (now I added[do not merge]), which help us to discover the real reason 😃.

Ahh, right - then please feel free to add labels on the right side of the page (now I added[do not merge]), which help us to discover the real reason 😃.

@airween, I cannot set labels. I thought it was clear from the start, or that's what I wrote on thewhy section when submitting the PR anyway.

Ahh, right - then please feel free to add labels on the right side of the page (now I added[do not merge]), which help us to discover the real reason 😃.

@airween, I cannot set labels.

uhm, sorry. Yes, now I see atriage access needs to apply any label (and let me take the opportunity again and call your attention your pending invitation to developers group :) - with that, you are able to apply labels)

I thought it was clear from the start, or that's what I wrote on thewhy section when submitting the PR anyway.

Sorry, I was a bit confused, because the sent modifications would have changed the build system.

Sorry again.

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud