Maintenance: cleanup the libsecurity API#2406
Open
yadij wants to merge 1 commit intosquid-cache:masterfrom
Open
Maintenance: cleanup the libsecurity API#2406yadij wants to merge 1 commit intosquid-cache:masterfrom
yadij wants to merge 1 commit intosquid-cache:masterfrom
Conversation
Replace some typedef with using keyword to cleanup the readability of grep output. Add C++17 [[maybe_unused]] attribute to remove need for (void) casting when no security library is is linked against. For cleaner code with warnings only when they actually matter. No logic changes.
rousskov
requested changes
Apr 12, 2026
Contributor
rousskov
left a comment
rousskov
left a comment
There was a problem hiding this comment.
This PR breaks CI build and likely to cause many conflicts with backlogged changes. A couple of other concerns are detailed in charge requests. I plan to come back to this after the backlog is dealt with.
|
|
||
| static bool | ||
| CreateSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &conn, Security::PeerOptions &opts, Security::Io::Type type, const char *squidCtx) | ||
| CreateSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &conn, Security::PeerOptions &opts, [[maybe_unused]] Security::Io::Type type, [[maybe_unused]] const char *squidCtx) |
Contributor
There was a problem hiding this comment.
With this change, code that should use, say, squidCtx but forgets to do so is no longer going to be flagged by the compiler. The improved esthetics are not worth losing those checks in those cases IMHO. The same concern probably applies to some other changes in this PR.
| debugs(83, DBG_IMPORTANT, "ERROR: " << squidCtx << ' ' << errAction << | ||
| ": " << (errCode != 0 ? Security::ErrorString(errCode) : "")); | ||
| #else | ||
| (void)ctx; |
Contributor
There was a problem hiding this comment.
ctx was not marked as "maybe unused", so its line should not be deleted from here AFAICT.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace some typedef with using keyword to cleanup the
readability of grep output.
Add C++17 [[maybe_unused]] attribute to remove need for (void)
casting when no security library is is linked against. For
cleaner code with warnings only when they actually matter.
No logic changes.