search

danburkert / procinfo-rs

A Rust library for accessing Linux process and system information
Apache License 2.0
91 stars 39 forks source link

Add parser for /proc/[pid]/maps #34

Open xmartinez opened 6 years ago

xmartinez commented 6 years ago

Hi! This adds support for parsing process memory mappings information from /proc/[pid]/maps.

xmartinez commented 6 years ago

Hi Dan!

Thanks a lot for taking the time to review this patch! (and sorry for the delay in answering, I have been sick for a couple of weeks).

I have incorporated most of your suggested changes in the update to the PR (thanks for the feedback, the code looks much better now).

The only pending point is the MapType {File, Anonymous} enum change to the API (see my comment above). Could you comment on my proposal, please? (I'd rather have your feedback before changing the API).

danburkert commented 6 years ago

Sounds good! No worries about the delay, I'm not always the quickest myself :)

xmartinez commented 6 years ago

Hi Dan!

I finally got around to change the API as per your suggestions. I have added a new MemoryMapKind enum that encodes the mapping type (file-backed, generic anonymous, vDSO, etc.).

I have also moved all fields that only make sense for file-backed mappings to a new FileMap struct, which is used by the MemoryMapKind::File enum variant.

I have added a file() -> Option<&FileMap> method to the MemoryMap struct to make client code that is only interested in file-backed mappings somewhat simpler (it avoids the necessity of using pattern matching in some use cases).

Apart from that, I have also extracted device number parsing onto its own function parse_dev() and the code now uses the device number to determine whether the mapping is file-backed (the previous approach used the inode number, which should also work, but comparing to the null device number will definitely work).

Let me know if there are any other changes that might improve the code. If not, I can rebase and squash the pull request branch (so it can be merged as a single logical commit).

ice1000 commented 5 years ago

Any update?