search

microsoft / msquic

Cross-platform, C implementation of the IETF QUIC protocol, exposed to C, C++, C# and Rust.
MIT License
3.92k stars 525 forks source link

Fixed lttng calls to not to cause compilation errors. #4366

Closed ManickaP closed 1 week ago

ManickaP commented 2 weeks ago

Fixes #4365

Not sure if this a right way to fix this, but it's the only one that worked for me. (I tried adding warning exceptions into CMake, but I don't understand it well enough to be able to make it work)

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.00%. Comparing base (f5bec53) to head (5c01f1e).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #4366 +/- ## ========================================== - Coverage 85.36% 85.00% -0.37% ========================================== Files 56 56 Lines 15426 15426 ========================================== - Hits 13169 13113 -56 - Misses 2257 2313 +56 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nibanks commented 2 weeks ago

As you can see from the CLOG / Validate failure, we cannot change these manually. They must be fixed upstream in microsoft/CLOG. I am curious why you are seeing these build failures, but our automation does not. The bug you opened doesn't seem to indicate what the difference might be.

ManickaP commented 1 week ago

I did a bit more digging and it might be GCC, I'm on 14 and the docs for 13 are slightly different for this error:

And Ubuntu 24.04 is on gcc 13: https://packages.ubuntu.com/noble/gcc

As usual, I'm not on a "supported" distro (Arch), but most of the time I'm just ahead with the packages and they get eventually propagated to other distros as well.

Please let me know if you're willing to accept this change or not. I'll put up PR into CLOG, but I don't want to spend time on this if this would get rejected.

nibanks commented 1 week ago

I agree with you, @ManickaP, that while we might not be hitting this yet, it's likely to come in the future, so it's good to get ahead of and fix it in CLOG. The project is mostly dotnet, so you (@ManickaP) probably can fix it faster than I can. 😄 Also, @chgray may have some time to help you with the CLOG stuff.

ManickaP commented 1 week ago

Closing, replaced by PR in CLOG: https://github.com/microsoft/CLOG/pull/112