Remove AclMatchedName from ACL::ParseAclLine() #239
Closed
Conversation
This file contains 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
The “ugly” use of AclMatchedName global supplied ACL name to error reporting during ACL (re)configuration. To solve this problem, we could pass ACL::name to the reporting code. This, however, is difficult because reporting code is located in stateless functions, such as aclDomainCompare() or acl_ip_data::NetworkCompare() without access to ACL::name. Another approach (implemented in this PR) is to reuse CodeContext concept for ACLs: the name of the ACL being parsed is appended to those debugs() automatically by CodeContext infrastructure.
to avoid noise when reporting context.
rousskov
requested changes
Nov 22, 2023
so that each parser (e.g., ACL::ParseAclLine()) could configure its context accordingly. Also configure ACL code context as early as possible.
that helped to undo the problematic cache_cf.cc context adjustments.
| @@ -0,0 +1 @@ | |||
| expect-message WARNING:.*is.a.subrange.of.*Because.of.this.*isignored | |||
There was a problem hiding this comment.
a simple way to catch mismatching instruction files
If we are going to keep to the "bad-" for negative tests, we can require that each "bad-" had an instruction file - that should be easy to adjust, I think. If positive tests (i.e., all other ones) do not need instruction files, then we can also prohibit such files for them.
rousskov
reviewed
Nov 30, 2023
test-suite/test-squid-conf.sh
Outdated
| @@ -99,6 +94,12 @@ then | |||
| exit 0; | |||
| fi | |||
|
|
|||
| if ! grep -q "# *define *\b$defineName\b $defineValue" $autoconfHeader | |||
| then | |||
| echo "$here: WARNING: Skipping $configFile test because $defineName is not $defineValue in $autoconfHeader"; | |||
There was a problem hiding this comment.
Please note that we want to keep the second parameter (i.e. $p2 and $defineValue) optional (rather than require it in all use cases).
rousskov
reviewed
Dec 19, 2023
There are similar official tests added at 0898d0.
After merge new tests became broken because some of them contained duplicated lines in '*.instructions' files. This happened because the uniqueness of these lines was previously ensured by adding a unique ACL name to each of them. This is not possible now because we removed AclMatchedName from parsing code). I had to adjust ACLs to make debugging unique without ACL name addition.
It was especially wrong to make that responsibility a secret. TODO: Do not hide c-string-to-SBuf conversion.
The function does not parse contexts. It calls some parser inside the given (parsing) context.
... addressing an earlier branch commit TODO.
... except where a single acl directive emits more than one warning.
|
This PR became official PR 1642. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The “ugly” use of AclMatchedName global supplied ACL name to error
reporting during ACL (re)configuration. To solve this problem, we could
pass ACL::name to the reporting code. This, however, is difficult
because the reporting code is located in stateless functions, such as
aclDomainCompare() or acl_ip_data::NetworkCompare() without access to
ACL::name.
Another approach (implemented in this PR) is to reuse CodeContext
concept for ACLs: the name of the ACL being parsed is appended to those
debugs() automatically by CodeContext infrastructure.