Open chansrinivas opened 2 months ago
I can code review this
I can code review this
Added in new mkfs.c for testing
I really like this project! The way the directory tree is displayed using graphs and symbols is very clear and intuitive. The code works impressively well for most cases, including recursive traversal with depth limits. The flags also can be combined and work together well without errors.
Suggestions for Improvement: 1.When an invalid flag is provided, the error message is unclear. For example, running /tree -Z results in: tree: cannot open -Z Instead, it could display an error message indicating an invalid flag.
2.The tree function opens the file descriptor multiple times when counting and printing files. To improve efficiency, it could handle both tasks in a single pass without reopening directories.
High Level Checks: ✅ PR fully resolves the task/issue ✅ Does the PR have tests? ✅ Does the PR follow project code formatting standards? ✅ Does the OS still compile and run? ✅ Is documentation included?
Code Checks: ✅ Does the code achieve its intended purpose? ❌ Were all edge cases considered? ✅ Is the code organized and maintainable? ✅ Does the code maintain the same level of performance? (no performance regressions)
Comments: I like your project! The output is very clean and easy to understand. Good comments in code, easy to follow logic. Overall, design is tailored for readability and clean functionality which I really enjoy as a reviewer. Suggestions for improvement:
I do think to improve efficiency you could potentially make some changes to how you process flags. As the earlier CR points out, runtime might be slow through the uses of recursively counting files/directories multiple times for one task, so a change I could recommend is making your status variables global. This way, you can check for them during your tree() and contains_valid_file() functions and hopefully get rid of the large while loop inside of tree() that seems to have the same or similar logic to contains_valid_file().
High Level Checks:
Code Checks:
This is a great rendition of the tree command and it supports a lot of options... great! Code doesn't quite follow xv6 conventions but is well organized.
Going through the code:
.
and ..
, etc. all repeat a few times and could be simplified with helper functions.When I view a tree with nested directories, the full path keeps getting printed. I have a small example:
├── ./dir1/
│ ├── ./dir1/test.c
│ └── ./dir1/a/
│ └── ./dir1/a/b/
usertrap(): unexpected scause 0x000000000000000f pid=3
sepc=0x0000000000000ec6 stval=0x0000000000003f98
To me, it makes more sense to show dir1
-> a
-> b
instead of the full path each time.
This brings me to the next issue I ran into: if I go deeper than 3 directories, this will crash, e.g., mkdir -p dir2/a/b/c/d/e/f/g
. Seems that it's crashing on a printf maybe?
Overall, this is a cool idea and I really like how complete it is as far as options go. You can earn +0.25 with the fixes above. Great job!
Team: Riya Mathur, Suhani Arora, Chandana Srinivas
This project is a command-line tool written in C that displays the directory structure in a tree-like format. It allows users to explore directories and subdirectories with options to filter files by extension, display file sizes, and show file and directory counts. Supports recursive traversal of directories.
Features:
/tree <directory>
- Display a directory tree structure for a given path (Default is.
which is the root directory)./tree . -F <file extension>
-Optionally filter files by their extension./tree . -S
- Optionally show the size of files in bytes./tree . -C
- Optionally count and display the number of files and directories in each directory./tree . -L <depth>
- Optionally limits the depth of the directory tree. Only shows directories and files up to the specified depth level.You could also multiple flags:
/tree . -C -L <depth>
- Shows the counts of files and limits the depth of the directory tree./tree . -S -F <file extension>
- Shows the file sizes in bytes and filters files by their extension.Since xv6 does not have directories by default, the updated mkfs.c will create two directories:
dir1
anddir2
within xv6 for more convenient recursive testing of directories.