Add tac command

[git547lab]

Supported patterns: tac FILE tac FILE -s separator tac -s separator FILE

(Separator is a non-empty quoted string, limited to no more than 5 spaces and 50 characters. Separators containing special characters (such as '(' and ')') may not be supported.)

Description:
tac FILE Print the lines in the specified file in reverse order.

tac FILE -s separator tac -s separator FILE Print from the first occurrence of the separator (inclusive) to the end of the file. Then, print from the beginning of the file to the position before the first occurrence of the separator.

[marcusaltman]

I'll code reviewing this one.

[kat-le]

I will also be code reviewing this!

[marcusaltman]

Issues:

1. You need a test program. I would also include a test file or have the test program create a test file. If you want to include the file, you'll need to add the file name to the Makefile on lines 137 & 138.
2. TAC should find the first instance of the separator starting from the end of the file instead of the beginning.
3. TAC should start the output on the character after the separator instead of the first character of the separator.
4. TAC with an empty string separator should print the file in normal order (like the CAT command).
5. The Linux TAC doesn't require quotation marks around the separator if it's a single string with no spaces.
6. The Linux command's "No such file" message is different.

Possible Issue: Every time I ran "make qemu" after "make clean" and "make", I got an error (see below), but when I ran the command a second time, it worked fine. You should see if it's doing the same thing on your computer. Mine has never done that before.

Error Message: riscv64-linux-gnu-ld: cannot find user/forktest.o: No such file or directory riscv64-linux-gnu-ld: cannot find user/ulib.o: No such file or directory riscv64-linux-gnu-ld: cannot find user/umalloc.o: No such file or directory riscv64-linux-gnu-ld: cannot find user/printf.o: No such file or directory make: *** [Makefile:104: user/usys.o] Error 1

Recommendations:

1. On line 153, you could do "while ((char_read = getline(&line, &sz, fd)) > 0)" and remove lines 155-161.
2. You don't need to reset the variables on lines 203 and 204. They'll be overwritten when you do the getline on line 155.
3. You can factor out the code on lines 177 and 188 by deleting line 177 and removing the else condition around line 188.
4. On line 191, instead of checking the values, you could just use them as booleans and say "if (has_separator && !match_found)"
5. On line 247, adding "\0" shouldn't be necessary since you initialized the new_str_arg array to all 0's on line 244 (0 = '\0').
   • If you keep the line, accessing the array directly with new_str_arg[strlen(arg)] = '\0' would be faster than strcpy

[kat-le]

Here are my suggestions:

• Line 238 and 281: You could also print out its usage just to provide more information and make it more clear.

• Line 297-298: checking the length of argv[i] and using strcmp to check if it matches “-s” is redundant. You can delete line 297 and change the for loop to: if (strcmp("-s", argv[i]) == 0){ }

• I tried running tac with a separator that appears multiple times in a file and it finds the first instance of the separator and prints until the end of the file. For example if the file contains “hello” on 2 different lines and I use that as the separator, it will print out the first instance of hello and treat the rest of the file as normal. This doesn’t break the program or anything but I am wondering if this was intended since I don’t think you have a test case that deals with this. You could leave it like that or if there are multiple separators in the file, then tac should treat each occurrence independently.

[malensek]

Nice work on this! The code is clean, works well, and is well-documented. While I agree that your test cases could be a program instead, you don't have to make that change for final submission. I would have you make the small improvements listed above (minor cleanups, simplifications) and beyond that this is perfect!

Future work: test suite. Not required for this submission.