Gave a learning task: compare sorting speeds. The program crashes when negative values are used in an array.
void countSort(int arr[], int n)
{
struct timeval ta, te;
gettimeofday(&ta, NULL);
int M = 0;
for (int i = 0; i < n; i++)
if (arr[i] > M)
M = arr[i];
int *countArray = (int *)calloc(M + 1, sizeof(int));
for (int i = 0; i < n; i++)
countArray[arr[i]]++;
for (int i = 1; i <= M; i++)
countArray[i] += countArray[i - 1];
int *outputArray = (int *)malloc(n * sizeof(int));
for (int i = n - 1; i >= 0; i--)
{
outputArray[countArray[arr[i]] - 1] = arr[i]; //segmentation fault
countArray[arr[i]]--;
}
for (int i = 0; i < n; i++)
arr[i] = outputArray[i];
free(countArray);
free(outputArray);
gettimeofday(&te, NULL);
printf("time: %lf sec\n", te.tv_sec - ta.tv_sec + (te.tv_usec - ta.tv_usec) / 1000000.0);
}
I tried rebuilding the function, making different starting points.
The program crashes when negative values are used in an array.
(Hint) When the input array contains a negative value, what do you suppose its corresponding index in countArray
will be?
And what is all this messy business with outputArray
? Don't you accumulate enough information in countArray
alone to update the original array without using a temporary?
Additionally, in C, do not cast the return value of malloc
, calloc
, and realloc
. Doing so is unnecessary (in C), and it can disguise certain errors.
Also, be careful that you are timing what you think you are timing. Memory allocation and deallocation are comparatively expensive, and you're doing both within the timed portion of your code. That might be exactly what you want, but it does complicate comparison of your results with algorithms that do not allocate. If you wanted to take the allocation out of the picture then one way would be to provide large-enough pre-allocated space for the function to work with.