Fixes #13 -- added sysctls - #17
Conversation
d20506a to
5ffa181
Compare
fe3c89e to
7eedce3
Compare
|
Good news... it compiles... and doesn't explode? Bad news: At this point I think this could use a second pair of eyes :-) |
|
I believe the buffers passed to your handler are kernelspace at that point, no? i.e. procfs has already done a read/write with userspace buffers. |
|
Well, that was embarassing! It works now though... sort of... if I do |
|
It works now! |
|
I'll hoist some of the seperable parts of this ( |
79bb4e7 to
2d1fabe
Compare
|
Ok, I think this is as minimized as I can reasonably go. |
geofft
left a comment
There was a problem hiding this comment.
LGTM after:
- the UB fix from #46, also confirming that my comments about concurrency safety are right
- is it correct to increment
pposif the write failed or partially failed? (It probably does not matter much)
A couple more questions:
- I guess we're boxing things because we need them to have a stable address and we can't have a self-referential struct? It'd be nice to figure out a way to avoid this, but that doesn't block landing this
- Does it make sense for
UserSlicePtrto implementcore::fmt::Read/core::fmt::Write? (Probably not because they work withstrand not[u8]?) - Does it make sense to make
trait SysctlStoragesymmetric in the two functions and have it take aUserSlicePtrReader? We already know that anything over a page is unreasonable so we can avoid an allocation. - Should we handle
pposbeing nonzero on a write?
|
a) Yes, box for stable address. b) It might make sense; I think c) Maybe? This seems simpler :-) d) Does something special need to be done on |
`pointer::add` and `pointer::offset` turn into a `getelementptr inbounds`, which is UB if it does not point to a valid object or one past a valid object (i.e., it enables compiler optimizations that make that assumption). Raw pointers to userspace are not pointers to valid objects. `pointer::wrapping_add` and `pointer::wrapping_offset` turn into a `getelementptr`, which is always defined (and so they're both safe).
Currently just has stubs for everything, not yet ready for review, using the WIP branch as a forcing function to review what I've written :-)