###Input format###
There'sThere are a few things I don't like about the way input is handled:
- Having all of your input done via the command line is awkward. Suppose you wanted to try a test case of 10 million integers. How would you even do that? I propose that you read your input from stdin instead. That way you could just create a test case input file and run your program like
countsort < input.txt
to test it. - I don't feel like the user should have to specify the maximum value as the second input value. Your program could just compute the maximum value as it reads in all the numbers. After all, your program doesn't need to use the max value until it is ready to do the sort. This would also allow you to remove some of the error checking you have where you test the array values against the maximum value.
- I don't think there is a need for the
[
and]
parts of the input, and also the comma separators. It would be simpler to just have the input be: the countn
followed byn
numbers without any extra brackets or commas.
###Overflow in size computation###
It looks like you took care of the special case where the maximum value was UINT_MAX
, because in your call to malloc()
you used the value max+1
and didn't want that to overflow to zero. However, there are many other ways to overflow in that same line:
unsigned int *counts = malloc(sizeof(int)*(maxval+1));
Suppose you were on a 32-bit platform with size_t
being 32-bits wide, and suppose maxval
were 0x40000000
. The expression sizeof(int) * (maxval+1)
would evaluate to 4 * 0x40000001
or just 4
because the result would be truncated to 32 bits. Note that 0x40000000
is just one example. Any value greater than or equal to 0x3fffffff
would cause problems for this platform.
###Bug: using uninitialized memory###
You allocate your counts
array using malloc()
, but you never clear your counts
array to 0 before you start to use it. So you could end up with garbage values for your counts. I would suggest using calloc()
so that your allocated array is cleared to zero automatically. A side benefit to using calloc()
is that it avoids your overflow problem because the arguments are split into a size and a count:
unsigned int *counts = calloc(maxval+1, sizeof(int));
###Overflow in counts###
If you were on a 64-bit platform with 64-bit size_t
and 32-bit unsigned int
, you could run into another type of overflow problem. Since your counts
array is of type unsigned int
(32-bit), your counts could overflow because your array could have more than 2^32 elements. For example, suppose your array had 5 billion instances of the number 5. Your program would overflow counts[5]
and get the wrong count in the end.