I'm writing counting sort in C. N is the number of elements in table which is to be sorted, k is max value that any of this element can be. However, this code, leaves me with the same table as the input. What's wrong?
void countingSort(int *tab, int n, int k) {
int *counters = (int *)malloc(k * sizeof(int));
int *result = (int *)malloc(n * sizeof(int*));
for (int i = 0; i < k; i++) {
counters[i] = 0;
}
for (int i = 0; i < n; i++) {
counters[tab[i]]++;
}
int j = 0;
for (int i = 0; i < k; i++) {
int tmp = counters[i];
while (tmp--) {
result[j] = i;
j++;
}
}
tab = result;
}
There are some problems in your code:
int *result = (int *)malloc(n * sizeof(int*));
uses an incorrect size. The array element type is int
, not int*
. You should write:
int *result = (int *)malloc(n * sizeof(int));
or better:
int *result = (int *)malloc(n * sizeof(*result));
note also that the cast is useless in C, unlike C++ where it is mandatory:
int *result = malloc(n * sizeof(*result));
you could avoid the extra initializing loop by using calloc()
:
int *counters = calloc(n, sizeof(*counters));
a major problem: the result array is never returned to the caller: tab = result;
just modifies the argument value, not the caller's variable. You should instead use the tab
array to store the results directly.
you do not free the arrays, causing memory leaks.
you do not test for allocation success, causing undefined behavior if memory is not available. You should return an error status indicating this potential problem.
Here is a corrected version:
// assuming all entries in tab are > 0 and < k
int countingSort(int *tab, int n, int k) {
int *counters = calloc(k, sizeof(*counters));
if (counters == NULL)
return -1;
for (int i = 0; i < n; i++) {
counters[tab[i]]++;
}
int j = 0;
for (int i = 0; i < k; i++) {
int tmp = counters[i];
while (tmp--) {
tab[j++] = i;
}
}
free(counters);
return 0;
}