10000 Factor out read_text_file_contents to ensure that data read from text files is a valid C string. by igorpeshansky · Pull Request #3359 · collectd/collectd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Factor out read_text_file_contents to ensure that data read from text files is a valid C string. #3359

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
Merged
merged 1 commit into from
Jan 7, 2020
Merged

Factor out read_text_file_contents to ensure that data read from text files is a valid C string. #3359

merged 1 commit into from
Jan 7, 2020

Conversation

igorpeshansky
Copy link
Contributor
@igorpeshansky igorpeshansky commented Dec 19, 2019 •
edited by mrunge
Loading

Also fixes a potential unterminated string in thermal_procfs_device_read.

ChangeLog: collectd: Factored out read_text_file_contents for reading text files

and used it to fix a potential unterminated string in the thermal plugin.

/cc @jkohen

[edit @mrunge]: Broke the ChangeLog line, since CI fails on long changelog lines.

@igorpeshansky
Copy link
Contributor Author

Note to the maintainers: this is a semantics-preserving refactoring, except for the potential bug fix in the thermal plugin. Since the bug hasn't been reported, AFAICS, I'm not sure what would be the value of adding it to the release notes, but I'm happy to add a ChangeLog line if necessary. Otherwise please tag as Unlisted Change.

@igorpeshansky igorpeshansky mentioned this pull request Dec 19, 2019
Merged
jkohen
jkohen approved these changes Dec 19, 2019
View reviewed changes
Copy link
Contributor
@jkohen jkohen left a comment

Choose a reason for hiding this comment

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

LGTM. I don't have approval rights.

@mrunge
Copy link
Member
mrunge commented Dec 20, 2019

Thank you!

The ChangeLog feature helps us with writing a changelog, which is also about to highlight and thank contributors.

If you don't want to be included there, I'm happy to flag this as unlisted feature. However, I'd prefer to add and list your contribution :) (I'd leave the ChangeLog question to you...)

@igorpeshansky
Copy link
Contributor Author

@mrunge That's fair. I assumed the ChangeLog was for user-visible changes only... Done.

This is ready for review.

mrunge
mrunge approved these changes Jan 7, 2020
View reviewed changes
Copy link
Member
@mrunge mrunge left a comment

Choose a reason for hiding this comment

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

thank you!

@mrunge mrunge merged commit 5ebc182 into collectd:master Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrunge mrunge mrunge approved these changes

@jkohen jkohen jkohen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@igorpeshansky @mrunge @jkohen
0