Unix command: sort (with support for flags)

[aajoseph2]

NAME sort - sort lines of text files

SYNOPSIS sort [OPTION] [FILE] [text output] | sort [OPTION] ex: ls | sort [OPTION]

DESCRIPTION Write sorted concatenation of FILE to standard output.

With no FILE, read standard input (not recommended).

Deafult sort is lexicographic.

Options:

-b: ignore leading blanks
-f: ignore case
-n: numeric sort; compare according to string numerical value
-r: reverse the result of comparisons
-u: output only unique values

-h: help page

AUTHOR Written by Shyon Ghahghahi and Amin Joseph

LIMITATIONS Limited support with multiple flags. Unwanted behavior when combining -n, -b, or -f

[AlexBareli]

Code Review:

Your sort project is very impressive and complex, and I think you all did an amazing job with the sort functionality overall. The code was well written, minimalistic and organized, and the majority of the code was well documented. While the flag functionality was a little buggy, I think it is still impressive that you were able to add flags to your program, and I think the way you handled them was very smart. You did forget the free the majority of the malloced memory in sortfunc.c, like on line 138, but I think is it impressive the number of different functions you were able to add. I know sort.c wasn’t the bulk of the project, but some adding some code comments there would be nice. Some major bugs that I’ve noticed was I tried running sort on mkdir and the program crashed, I don’t know if there is a limit to the file size that you can sort. Also, I know this was mentioned in your doc comments, but the program does have some unexpected behavior if provided with no arguments, and I think it would be better to check to see if the user has inputed the correct number of arguments or not. Overall fantastic work on this complex project!

Summarized Suggested Changes:

• Doing sort on large files crashes the program, in my example I tried running sort on one of the user space programs.
• sortfunc.c has malloced memory that hasn’t been freed, which can cause issues.
• Sort without arguments provides unexpected behavior, it is better to check to see if the user has provided any arguments.

[john-la-usfca-98]

Code review: Your code looks really good, I do see a lot of hard work behind the code. Some of the thing that I really like here is:

1. You have made the flag options for alternative cases just like how flags works in linux command line.
2. You have made seperate cases to sort out the file, instead of just be like, 1 case do all.

Some of the thing I find it a bit off, and maybe some suggestions:

1. From my own personal experience, when I first tried your code, I just type sort and enter, and it causes to read input infinitely, with no way of constraint or stop the input reading process. I know that you have mentioned that sorting from input is not recommended, but I think it's best if you just throws argument error, and requires the user to have to input correctly, instead of leaving it dangling like that. This, in my opinion, is really important if we're (hypothetically) going to make FogOS run from BIOS as a full OS, not just from QEMU virtual machine.
2. Based on how I see the code here, your insertion sort have a runtime of O(n^2), due to a for loop (i++) and a while loop (j--). This is not the optimal runtime, as the bigger the text file, the longer the runtime is, and it can cause crashes if it gets way too much. My suggestion would be to use a different sort, like quicksort for bigger dataset or shell sort (derived from insertion sort, but doesn't take much space).
3. There are only 1 toLower function that uses malloc, but you forgot to deallocate correctly. My suggestion would be to make sure you use malloc to allocate properly more on all the other functions, instead of just the only toLower, and remember to deallocating it.

This is just me being nitpicking, but all and all, great job.

[malensek]

Nice work. It's extremely hard to find any faults with this project; please review the comments above and make fixes if necessary. I wouldn't say the infinite reading is a problem when run without args, ^D should terminate it correctly. You implemented several C library or useful utility features here, they'd be great if spun off to a separate library someday (not a project for you two to worry about!).