-
Notifications
You must be signed in to change notification settings - Fork 49
Merge ps implementation back to main #83
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
base: main
Are you sure you want to change the base?
Conversation
|
I would like to code review this. |
|
I'll review this :) |
|
High-Level Checks: [✅] Does the PR fully resolve the task/issue? [✅] Are there tests included? [✅] Does the code follow project formatting standards? [✅] Does the OS compile and run successfully? [✅] Is documentation provided? Code Checks: [✅] Does the code accomplish its intended purpose? [✅] Were all edge cases considered? [✅] Is the code well-organized and maintainable? [✅] Does the code maintain performance? |
|
Great idea for a P1-scope project, excellent execution. Your documentation is well-written, code is clean, concise, and well-documented, and overall... it's hard to find anything to complain about. Printing to stderr for error messages? (I guess I could complain about readability with respect to using the ternary operator, but you could probably just tell me to "get good" and that'd be enough). Great work! You acknowledge this in your code doc, but one small issue is the amount of data that will need to be transferred out to user space due to allocating space for the entire process table (mostly empty in our use cases). On Linux, this is a non-issue because this information is reported via procfs instead. On the BSDs, they usually have a system call similar to yours but dynamically resize the process, copy out the exact amount of necessary data, copy out its size (as an "out arg"), and then update the pointer it was given to point at the freshly-allocated data. Too much work for P1. :-) |
Issue Link #51
Implemented:
1. kernel level changes
2. user level changes
3. documentation changes
Implementation details
How to test
Note: automated testing is incomplete due to lack of shell scripting support in this release of xv6