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.

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

/proc/sysvipc/shm #154

Merged
merged 1 commit into from Nov 10, 2021
Merged

/proc/sysvipc/shm #154

merged 1 commit into from Nov 10, 2021

Conversation

tatref
Copy link
Contributor

@tatref tatref commented Oct 18, 2021

This is related to my last commit to add /SYSV paths to the memory maps (#151)

Output of the example:

key: 0, shmid: 4
============
2262: ["/usr/bin/gnome-shell"]
2284: ["/usr/bin/Xwayland", ":0", "-rootless", "-terminate", "-accessx", "-core", "-listen", "4", "-listen", "5", "-displayfd", "6"]

I check with Oracle databases that make huge usage of shared memory, it works as expected.

This PR seems good to me, but I wouldn't mind somebody else checking the types of the fields in the Shm struct as I'm not very fluent in C.

I'll also try add comments to theses fields

let mut vec = Vec::new();

// See printing code here:
// https://elixir.bootlin.com/linux/latest/source/ipc/shm.c#L1737
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for including this reference. Having comments like this are always helpful

@eminence
Copy link
Owner

With a quick skim, this looks OK to me. I would be nice to have all the fields of Shm documented, but it's not required for a first version.

@tatref
Copy link
Contributor Author

tatref commented Oct 28, 2021

I added doc for every items, hopefully it's correct!

@eminence
Copy link
Owner

Can you rebase this on top of the latest master branch?

@eminence
Copy link
Owner

eminence commented Nov 6, 2021

What do you think is left for the PR? Is it still a "WIP" ?

@tatref
Copy link
Contributor Author

tatref commented Nov 6, 2021

I fixed some comments in the example file, I think this PR is finished. You can merge it if it's good for you.

@eminence eminence changed the title WIP: /proc/sysvipc/shm /proc/sysvipc/shm Nov 6, 2021
Err(_) => continue,
}
}
println!("");
Copy link
Contributor

Choose a reason for hiding this comment

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

use println!(); instead

Copy link
Contributor

Choose a reason for hiding this comment

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

clippy::println_empty_string

@@ -0,0 +1,33 @@
#![allow(clippy::print_literal)]
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this line

Ok(memory_maps) => {
for (memory_map, memory_map_data) in &memory_maps {
match memory_map.pathname {
procfs::process::MMapPath::Vsys(key) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

enum variant MMapPath::Vsys not found?

Copy link
Contributor

Choose a reason for hiding this comment

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

MMapPath::Vsys compile error(not found)

origin  https://github.com/tatref/procfs.git (fetch)
origin  https://github.com/tatref/procfs.git (push)
commit a0679242e613e4618cb92a0ed70721af218efa7d (HEAD -> sysvipc_shm, origin/sysvipc_shm)
Author: tatref <tatref@users.noreply.github.com>
Date:   Sat Nov 6 19:13:11 2021 +0100

    Update shm.rs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you're missing #151

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rebased this PR from current master

@pymongo
Copy link
Contributor

pymongo commented Nov 9, 2021

I think you should squash five commit to one commit

@eminence eminence merged commit 6d3c4ae into eminence:master Nov 10, 2021
@eminence
Copy link
Owner

Thanks!

@eminence eminence added this to the v0.12.0 milestone Nov 26, 2021
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.

None yet

3 participants