-
Notifications
You must be signed in to change notification settings - Fork 49
added df command #96
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?
added df command #96
Conversation
|
I will code review this. |
|
I can code review this |
|
fs.c df.c other utils are good Most files are missing comments on the purpose of the new functions and structs. High-Level Checks: |
|
I can code review this! |
|
High Level Checks:
Code Checks:
Cool project! It's pretty straightforward and informative- an edge case that I found is when I put an invalid flag like df -p, it prints out the output for df, but I think there could be a more specific error message that says "flag doesn't exist" or something. For future advancements, you could probably add more error handling in terms of hardware issues the computer itself faces. Your code style and documentation looks good to me! |
|
I think it's great that your project has made some changes to kernel space. There are a few problems with what is being claimed in the PR and what was proposed in the original issue and some code cleanup / refactoring is necessary. I'm quite surprised how much you know about the xv6 file system code, so at some point it'd be great to do a one-on-one code review on this so I can pick your brain.
This does not seem to be implemented. xv6 doesn't have a concept of file system mount points so I'm a bit confused how you would claim you support this. It's even in the usage: But not anywhere in the code. In the original issue (#63) it says would have some code to traverse from subdirectories up to the root of the fs.
This is not actually supported! I don't see any code for conversions. What if I up the FS size to 1 GB? It looks like everything is just divided by 2.
Again, this isn't something that xv6 has. It has one FS type. The first two things should've been caught by code reviewers. (-0.5 for both) Code fixes:
You can fix the style/doc/organization issues above to get +1 back on your current score. |
The
dfcommand provides filesystem statistics such as total disk size, used space, available space, and inode information.Key Features:
Basic Disk Usage Reporting:
The df command displays total blocks, used blocks, and free blocks for the entire filesystem. When run without arguments, it reports on all mounted filesystems. If one or more paths are provided, it reports only on the filesystems associated with those paths.
Human-Readable Output (-h option):
Outputs sizes in KB, MB, GB, etc., making it easier to understand large filesystems.
Inode Usage Information (-i option):
Displays inode usage instead of block usage, providing insight into the number of inodes available and used.
Usage:
System Call Changes:
Added the statvfs system call to gather filesystem statistics from the kernel. This system call provides the df command with information about total blocks, free blocks, used blocks, total inodes, and free inodes.
Future Development:
Add filters to report only on specific filesystem types (e.g., ext2, ext3, etc.).
Implement color-coded output to highlight filesystems that are nearing capacity.