Adding tree command to FogOS

[chansrinivas]

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 and dir2 within xv6 for more convenient recursive testing of directories.

[andywu42]

I can code review this

[ajsdkty3]

I can code review this

[chansrinivas]

Added in new mkfs.c for testing

[ajsdkty3]

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)

[andywu42]

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:

1. Output for parsing flags should be more informative towards the flag being processed incorrectly
   Ex: /tree -c
   Output: tree: cannot open -c
   Suggestion: “Invalid flag”
   Ex: /tree . -F
   Output: tree: cannot open -F
   Suggestion: “Expected value for -F”
2. The parsing of values for flags -F and -L should have some informative errors when nothing or something wrong is read after these flags.
   Ex: /tree . -F hi
   Output: (Nothing)
   Suggestion: Print something like “Invalid value for -F flag. File extensions should start with ‘.’ ”
   Ex: /tree . -L hi
   Output: ./
   Suggestion: add if case to check for value, if nothing/non-int print something like “Invalid value for -L flag.”
3. Edge case not checked: -S and -C flags used together
   The output of -S is at the end of the file, with the output of -C at the end of directories, I would expect both should be able to run at the same time? (I notice it runs -C over -S no matter what)
   Ex: /tree dir2 -S -C
   Output:
   dir2/ [0 directories, 1 files]
   Expected:
   dir2/ [0 directories, 1 files]
   └── dir2/test.txt (size: 6 bytes)
4. Cleanup: remove test.txt in root branch (different from the one in dir2), it is not used or displayed when running make qemu (or add it to the Makefile)

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:

• 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)

[malensek]

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:

• there's a lot of repeated logic. your directory reading code, checking for . and .., etc. all repeat a few times and could be simplified with helper functions.
• error messages should go to stderr instead of stdout
• as mentioned above the flag parsing is pretty good but needs more error handling.

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!