Skip to content

Conversation

@HenrikanHa
Copy link

@HenrikanHa HenrikanHa commented Oct 4, 2025

Summary
We implemented: mv (move/rename files) and rmdir (remove empty directories), along with their respective automated test programs mvtest and rmdirtest.


Features Implemented
mv

  • Moves or renames files.
  • Supports moving files between directories (with or without trailing slashes).
  • Prevents moving a file to itself.
  • Handles overwriting and confirmation prompts via:
    • -f → force overwrite
    • -i → interactive confirmation before overwrite
    • -v → verbose mode (prints 'SRC' -> 'DST')

rmdir

  • Removes empty directories.
  • Refuses to remove non-empty directories or non-directory paths.
  • Supports:
    • Removing multiple directories at once
    • -v flag to print a confirmation message when directories are removed

Build Instructions

To build:
$ make clean
$ make qemu
or:
$ make clean && make qemu
This will compile both commands (mv and rmdir) and their corresponding test programs (mvtest, rmdirtest).
You can verify the binaries are included by running:
$ ls
You should see entries like:
mv
mvtest
rmdir
rmdirtest


Automated Tests
mvtest
Validates:

  • Simple file moves and renames
  • Force overwrite (-f)
  • Verbose mode output (-v)
  • Moving files into and between directories
  • Handling of trailing slashes in directory paths
  • Detection of same-file moves

rmdirtest
Validates:

  • Removing single and multiple empty directories
  • Refusing to remove non-empty directories
  • Refusing to remove files that are not directories
  • Printing correct output in verbose mode (-v)

Documentation
Added Markdown documentation:

  1. docs/mv_README.md – build instruction, usage, examples, and explanation of flags and behaviors
  2. docs/rmdir_README.md – build instruction, usage, examples, and test output
    Each document includes both manual examples and expected test outputs.

How to Test
After building the user programs, run:
$ mvtest

Expected results:
Test: simple move... OK
Test: -f overwrite... OK
Test: -v verbose output... OK
Test: move into directory... OK
Test: same file detection... mv: 's.txt' and './s.txt' are the same file
OK
Test: move into dir with trailing slash... OK
Test: move file between directories... OK
Manual test: run mv -i src dst and try typing y/n yourself.

$ rmdirtest
Expected results:
Test: remove one empty directory... OK
Test: remove multiple directories... OK
Test: refuse non-empty directory... rmdir: full: directory not empty
OK
Test: refuse non-directory... rmdir: afile.txt: not a directory
OK
Test: -v prints confirmation... OK
Manual: try rmdir with multiple args or flag -v yourself in the shell.

@HenrikanHa HenrikanHa closed this Oct 4, 2025
@HenrikanHa HenrikanHa reopened this Oct 4, 2025
@DemetriusChatterjee
Copy link

On make and make qemu ran fine. Mvtest performed well, providing clear output to inform the user, and additional commands were provided at the end. Should the user wish to manually test, the "mv -i" command is effective. Rmdir works perfectly. The documentation is excellent, and there are also test programs available for a quick demo. The code formatting is generally consistent and follows the style of the other user-space utilities in the project. There are no major formatting issues to address. The code performs its intended function. Good Job!

High Level Checks:
[x] PR fully resolves the task/issue.
[X] Does the PR have tests?
[X] Does the PR follow project code formatting standards?
[X] Does the OS still compile and run?
[X] Is documentation included?

Code Checks:
[x ] Does the code achieve its intended purpose?
[x] Were all edge cases considered?
[x] Is the code organized and maintainable?
[x] Does the code maintain the same level of performance?

@ccarter2304
Copy link

The functionality worked as expected from following the documentation. There weren't any outstanding formating issues within the files, everything seemed cohesive. The documentation was laid out great, with how to build and test the code. I do not think there are any improvements that need to be made! Great job!

High Level Checks:

  • [ X] Is documentation included? This should explain how to build, run, and test the software.
  • [ X] PR fully resolves the task/issue
  • [ X] Does the PR have tests?
  • [ X] Does the PR follow project code formatting standards?
  • [ X] Does the OS still compile and run?

Code Checks:

  • [ X] Does the code achieve its intended purpose?
  • [ X] Were all edge cases considered?
  • [ X] Is the code organized and maintainable?
  • [ X] Does the code maintain the same level of performance? (no performance regressions)

@aryasri16
Copy link

aryasri16 commented Oct 11, 2025

Great job with documentation and formatting! I really like how you added automated tests to your repo, that was really helpful as a first time viewer. I was able to use the -f, -i, and -v flags just fine for mv. I also found that rmdir works perfectly as well. I don’t think I have suggestions to improve readability, efficiency, or correctness because I found it all very intuitive.

High Level Checks:

  • [ X] Is documentation included? This should explain how to build, run, and test the software.
  • [ X] PR fully resolves the task/issue
  • [ X] Does the PR have tests?
  • [ X] Does the PR follow project code formatting standards?
  • [ X] Does the OS still compile and run?
    Code Checks:
  • [ X] Does the code achieve its intended purpose?
  • [ X] Were all edge cases considered?
  • [ X] Is the code organized and maintainable?
  • [ X] Does the code maintain the same level of performance? (no performance regressions)

@malensek
Copy link
Contributor

This looks great! Nice work. Here are some things I noticed as I went through this:

  • MV_MAXPATH is 256 but xv6 MAXPATH is 128. I doubt we are going to work with such long pathnames, but still, this means that the kernel will ignore half of the path you are passing in if it's too long.
  • Error messages / usage should be printed to stderr instead of stdout
  • This is essentially running mv with -n all the time (no clobber). I get that you wanted to implement -f force so your program behavior kind of makes sense, but I would probably have added -n as well since you already support it.
  • I am surprised that mv does not rename directories, since regular mv does. It would be a bit clunky to do, but you can modify the directory entries for that.

I am a bit surprised that the reviewers above weren't able to come up with any more suggestions for you. It's a very high quality project but there's always more we can do (I'm only saying this so we can feel like we have job security).

5/5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants