search

johalun / sysctl-rs

A simplified Rust interface to the sysctl system call
MIT License
51 stars 23 forks source link

New design #24

Closed LuoZijun closed 5 years ago

LuoZijun commented 5 years ago

Hi, I have made a lot of modifications and simplifications based on this library, and added support for Linux systems. Although @johalun mentioned that he is doing some work in this area, I can't seem to wait any longer :)

If you are interested in this version of my revision, I would be happy to submit a PR.

https://github.com/ExodusVPN/exodus/tree/rewrite/crates/sysctl

https://github.com/ExodusVPN/exodus/blob/rewrite/crates/sysconfig/src/sysctl.rs

johalun commented 5 years ago

Hey! I already have a working solution committed. Please check it out (master branch). What missing is updated docs and tests but I'm working on that and plan to release a new version once complete.

LuoZijun commented 5 years ago

CodeLine: https://github.com/johalun/sysctl-rs/blob/master/src/linux/ctl.rs#L99-L101

It seems that you treat all the return value types as Node/String. If we maintain a Table in the userland, it is used to indicate the return value type of a CTL_KEY and other meta information. Not better?

Example:

https://github.com/ExodusVPN/exodus/blob/rewrite/crates/sysctl/src/sys/linux.rs#L47

johalun commented 5 years ago

It's a good idea but sounds like a terrible maintenance burden and you'll never know what sysctl's are added in the future, or added by private kernel modules. Having a cross platform compatible way to simply access all sysctls as string make applications simpler as well. The same sysctl might have different type on unix vs linux.

LuoZijun commented 5 years ago

it makes sense :)

i will close PR #23 , and this issue.