Fixed location reporting and long lines in config errors #3383
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
|
Build SUCCESS |
szemere
reviewed
Aug 10, 2020
szemere
reviewed
Aug 10, 2020
|
I added the "user-visible-feature" flag, since we are changing the output. However I think this one do not need extra documentation in the admin guide. With a news file, it is approved from my sight. |
|
Looks good to me too. @bazsi please add the news file. Then this can go in. |
Sometimes it is a big deal to see how syslog-ng parsed a specific token when trying to understand an error message. Together with the fixed location reporting in the followup patch, I hope this makes a couple of bugreports avoidable. Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
… reporting errors Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
… token When the error reporting format was improved to include the entire include stack, we started using location information from the lexer's state, which may not be entirely correct. The lexer sometimes is advance to the token triggering the error (because of LALR or simply because we are erroring on the first token of a rule), in these cases using the location information in the lexer is incorrect, and we should use the yylloc value as passed by the grammar. This fixes cases where the caret showing the exact error location points a bit past to the token that causes the error, for instance: @Version: 3.29 rewrite foo { subst ("", "", value("MESSAGE") type("pcre") flags("global store-matches")); }; This was reported as: 3 rewrite foo { 4-----> subst ("", "", value("MESSAGE") type("pcre") flags("global store-matches")); 4-----> ^ 5 }; 6 Please note that the caret points to the closing parenthesis instead of the incorrect flag. With the fix we get: 1 @Version: 3.29 2 3 rewrite foo { 4-----> subst ("", "", value("MESSAGE") type("pcre") flags("global store-matches")); 4-----> ^^^^^^^^^^^^^^^^^^^^^^ 5 }; 6 This would have saved us at least one bug-report where the user didn't understand the error he was getting. Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
bazsi
force-pushed
the
fixed-location-reporting-and-long-lines-in-config-errors
branch
from
af83364
to
695407c
Compare
|
Build SUCCESS |
szemere
previously approved these changes
Aug 18, 2020
Signed-off-by: Antal Nemes <antal.nemes.hu@gmail.com>
furiel
approved these changes
Aug 18, 2020
|
Build SUCCESS |
szemere
approved these changes
Aug 18, 2020
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.
This branch fixes a couple of issues in configuration error reporting:
+1 I've added the parsed value to the main grammar error message for cases where the value is a string.
All this together would probably have avoided the bug report in #3382