Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Add mountinfo module #123
Conversation
zyga
added some commits
Sep 2, 2016
tyhicks
reviewed
Sep 2, 2016
| + return NULL; | ||
| +} | ||
| + | ||
| +void cleanup_mountinfo(struct mountinfo **ptr) |
zyga
Sep 2, 2016
Collaborator
This is a library function (for users of mount info), note that free_mountinfo is static/private.
tyhicks
reviewed
Sep 2, 2016
| + return info; | ||
| +} | ||
| + | ||
| +static struct mountinfo_entry *parse_mountinfo_entry(const char *line) |
tyhicks
Sep 2, 2016
Collaborator
As I mentioned in IRC, this function is much more complicated than I'd like it to be. However, it looks to be correct and I don't have a better suggestion at this time.
tyhicks
reviewed
Sep 2, 2016
| + int total_used = 0; | ||
| + char *parse_next_string_field() { | ||
| + char *field = &entry->line_buf[0] + total_used; | ||
| + nscanned = sscanf(line + total_offset, "%s %n", field, &offset); |
tyhicks
Sep 2, 2016
Collaborator
Could you add a comment about what's going on here with field? It takes a while to realize that the parsed string is being stored in the array at the end of mountinfo_entry, which is being allocated to the size of the passed in line. It is also worth mentioning that the strings stored in that array are delimited with NUL characters from the calloc(). This is quite confusing and is going to take anyone reading the code quite a bit a of time to digest without a comment.
tyhicks
reviewed
Sep 2, 2016
| + char *super_opts; | ||
| + | ||
| + struct mountinfo_entry *next; | ||
| + char line_buf[0]; // Buffer holding all of the text data above; |
tyhicks
Sep 2, 2016
Collaborator
How about a comment saying not to add any new fields after .line_buf? Someone not well versed in C might not understand.
|
Ack after adding the requested comments. Thanks! |
zyga
added some commits
Sep 2, 2016
|
Thanks for the added comments. I'm happy with this PR. |
Conan-Kudo
reviewed
Sep 3, 2016
| - // result is similar to using strtok except that the source and destination | ||
| - // buffers are separate. | ||
| + // The parsing code below, specifically parse_next_string_field(), uses | ||
| + // this extra memory to hold data parsed from the original line. In the end |
Conan-Kudo
reviewed
Sep 3, 2016
| - // all of the text fields. | ||
| + // NOTE: the mountinfo structure is allocated along with enough extra | ||
| + // storage to hold the whole line we are parsing. This is used as backing | ||
| + // store for all of the text fields. |
Conan-Kudo
commented
Sep 3, 2016
•
Edited 1 time
-
Conan-Kudo
Sep 3, 2016
-
Sep 3, 2016
|
Aside from the couple of comments I made about your typo fixes, it looks good to me. |
zyga
added some commits
Sep 3, 2016
Conan-Kudo
commented
Sep 3, 2016
|
|
zyga commentedSep 2, 2016
•
Edited 1 time
-
zyga
Sep 2, 2016
This patch adds module capable of parsing /proc/self/mountinfo.
Snap-confine needs to parse this file to understand what is currently mounted in a way
that makes bind mounts visible (classic /proc/mounts is insufficient for this purpose).
For extra details about this topic see: https://lwn.net/Articles/265920/
Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com