8000 syslog: Replace fstream with custom non-blocking getline by theopolis · Pull Request #6539 · osquery/osquery · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
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.

Sign up for GitHub

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

Jump to bottom

syslog: Replace fstream with custom non-blocking getline #6539

Merged
merged 4 commits into from
Jul 14, 2020
Merged

syslog: Replace fstream with custom non-blocking getline #6539

merged 4 commits into from
Jul 14, 2020

Conversation

theopolis
Copy link
Member

This is an initial attempt to take the awesome work from #5969 and #5232, and minify it. The original bug report is #4810.

The goal is to have a non-blocking, mostly accurate, fast enough, minimalistic std::getline.

There may be areas for improvement from a performance and accuracy point of view. Areas to consider, copying data from the internal buffer out to the caller, line 106; the rare/exceptional case of shifting any left over data, line 110; giving up trying to handle a line larger than 2048 bytes, line 96; others?

@theopolis theopolis added bug Linux events Related to osquery's evented tables or eventing subsystem labels Jul 2, 2020
@theopolis theopolis closed this Jul 3, 2020
@theopolis theopolis reopened this Jul 3, 2020
@theopolis theopolis added this to the 4.5.0 milestone Jul 4, 2020
directionless
directionless previously approved these changes Jul 10, 2020
View reviewed changes
Copy link
Member
@directionless directionless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay tests. Code seems fine, I'm curious if you think there's a race in there and if it's something to fix. But happy to defer to you if you don't think we need it

osquery/events/linux/syslog.cpp Show resolved Hide resolved
osquery/events/linux/syslog.cpp Show resolved Hide resolved
@theopolis theopolis closed this Jul 11, 2020
@theopolis theopolis reopened this Jul 11, 2020
@theopolis
Copy link
Member Author

@directionless, if you have time, I addressed your comments in the original review.

directionless
directionless approved these changes Jul 13, 2020
View reviewed changes
Copy link
Member
@directionless directionless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume there's an implicit unlock at the end of the routine

@theopolis theopolis merged commit 3022689 into osquery:master Jul 14, 2020
This was referenced Jul 14, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@directionless directionless directionless approved these changes

Assignees
No one assigned
Labels
bug events Related to osquery's evented tables or eventing subsystem Linux
Projects
None yet
Milestone
4.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@theopolis @directionless
0