Skip to content

Update mountinfo parsing#228

Merged
SuperQ merged 1 commit into
masterfrom
superq/mountinfo
Nov 8, 2019
Merged

Update mountinfo parsing#228
SuperQ merged 1 commit into
masterfrom
superq/mountinfo

Conversation

@SuperQ
Copy link
Copy Markdown
Member

@SuperQ SuperQ commented Oct 25, 2019

  • Use a regexp to parse the mountinfo lines.
  • Use util.ReadFileNoStat() to instead of a buffered reader.
  • Add a test for hypens in the mountpoint.

Fixes: #225

Signed-off-by: Ben Kochie superq@gmail.com

@SuperQ SuperQ requested a review from pgier October 25, 2019 15:42
@discordianfish
Copy link
Copy Markdown
Member

Hrm not sure that using a regex is the best way. How do the usual linux tools parse this? I'd assume there is some regex-free way to do that. If not, this is fine.

@SuperQ
Copy link
Copy Markdown
Member Author

SuperQ commented Oct 28, 2019

The only code I've found so far was some random Perl code. It splits on spaces, which might be a reasonable way to go. But I thought the regexp would be a little better for validating the contents of the various sections.

@discordianfish
Copy link
Copy Markdown
Member

I'd argue we don't want to validate this at all.

See: http://man7.org/linux/man-pages/man5/proc.5.html
I think the right way to parse this is:

  • split by space
  • store elements 1-6 individually
  • start looping over element 7+ until you find a -
  • store remaining elements individually

@SuperQ
Copy link
Copy Markdown
Member Author

SuperQ commented Oct 29, 2019

Ok, I'll re-work this.

@SuperQ
Copy link
Copy Markdown
Member Author

SuperQ commented Oct 29, 2019

@discordianfish Ok, now it's a bunch of much simpler offsets and loops. Thanks for the suggestion.

@SuperQ SuperQ force-pushed the superq/mountinfo branch 3 times, most recently from 243bccd to 44dcc29 Compare October 30, 2019 08:45
@SuperQ
Copy link
Copy Markdown
Member Author

SuperQ commented Oct 30, 2019

I cleaned up a couple more things, I feel like the optional field parsing is now easier to understand, and less buggy.

Copy link
Copy Markdown
Collaborator

@pgier pgier left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread mountinfo.go Outdated
@discordianfish
Copy link
Copy Markdown
Member

Beside that, LGTM

@SuperQ
Copy link
Copy Markdown
Member Author

SuperQ commented Nov 6, 2019

Ok, I returned the optional field check to ignore not fail.

* Rework string handling to better loop over values.
* Use util.ReadFileNoStat() to instead of a buffered reader.
* Add a test for hypens in the mountpoint.

Fixes: #225

Signed-off-by: Ben Kochie <superq@gmail.com>
@SuperQ SuperQ merged commit 5593693 into master Nov 8, 2019
@SuperQ SuperQ deleted the superq/mountinfo branch November 8, 2019 12:06
remijouannet pushed a commit to remijouannet/procfs that referenced this pull request Oct 20, 2022
* Rework string handling to better loop over values.
* Use util.ReadFileNoStat() to instead of a buffered reader.
* Add a test for hypens in the mountpoint.

Fixes: prometheus#225

Signed-off-by: Ben Kochie <superq@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parsing hypen in mountinfo incorrect

3 participants