-
Notifications
You must be signed in to change notification settings - Fork 462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
F/syslog parser fixes #1206
Merged
Merged
F/syslog parser fixes #1206
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
This function can be used to copy MsgFormat related options during for instance a _clone() operation. Such an example follows in the syslog-parser() as the subsequent patch. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
The syslog parser supports msg-format related options and albeit the code has a lot of smells in the area (beginning with all the syslog related settings in MsgFormatOptions), we should make an effort to use them. To do that, we need to set defaults, and initialize/destroy these options properly. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
The LM_TS_STAMP is already filled by the time we get to syslog-parser(), however we do want to fully parse the date. One case that was missing was when the input timestamp didn't have a timezone and the syslog-parser() did have a time-zone() option specified. The correct behaviour is to use the timezone specified in options, however since it was already parsed earlier, it wasn't the case. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
@csabamajor, I think this is of interest to you. |
@@ -32,10 +32,22 @@ syslog_parser_process(LogParser *s, LogMessage **pmsg, const LogPathOptions *pat | |||
LogMessage *msg; | |||
|
|||
msg = log_msg_make_writable(pmsg, path_options); | |||
|
|||
msg->timestamps[LM_TS_STAMP].tv_sec = -1; | |||
msg->timestamps[LM_TS_STAMP].zone_offset = -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And what about tv_usec
?
Otherwise seems good. |
Signed-off-by: Laszlo Budai <stentor.bgyk@gmail.com>
Signed-off-by: Laszlo Budai <stentor.bgyk@gmail.com>
Signed-off-by: Laszlo Budai <stentor.bgyk@gmail.com>
@bazsi: I've updated your branch with some changes |
kvch
approved these changes
Sep 20, 2016
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 contains two pretty important fixes to syslog-parser():
all structured data elements were parsed as the empty string (due to an error in MsgFormatOptions initialization)
partial timestamps were not always parsed properly
Please merge!