qsort does nothing with cmpstr

Question

I'm not sure why qsort doesn't change my array because the elements aren't in alphabetical order. Can someone help me figure out what I'm doing wrong.

char **wordlist = malloc(sizeof(char*));
int i, numwords = 0;
wordlist[0] = strdup(words[0]);

for(i = 0; i < wcount; i++)
{
    wordlist = realloc(wordlist, (numwords+1)*sizeof(char *));
    wordlist[numwords] = strdup(words[i]);
    numwords++;
}

printf("Added %d words to the array and they are:\n", numwords);
for(i = 0; i < numwords; i++)
{
    printf("%s\n", wordlist[i]);
}

qsort(wordlist, numwords, sizeof(char *), cmpstr);

for(i = 0; i < numwords; i++)
{
    printf("%s\n", wordlist[i]);
}

int cmpstr(const void* a, const void* b)
{
   const char* aa = (const char*)a;
   const char* bb = (const char*)b;
   return strcmp(aa, bb);
}

The `realloc` is wrong. You should do `realloc(wordlist, (numwords+2)*sizeof(char *));` — iBug, Nov 15 '17 at 03:03
`wordlist[0]` is assigned twice. `wordlist[0] = strdup(words[0]);` and first `wordlist[numwords] = strdup(words[i]);`. Thus losing first allocation. — chux - Reinstate Monica, Nov 15 '17 at 03:05
Wrong compare function. `a` and `b` **point** to `const char *` — chux - Reinstate Monica, Nov 15 '17 at 03:07
Not a knock, but generally when you say "*A well tested C-library function behaves wrong*"... it's not the function that is to blame. You are looking for `return strcmp (*(char * const *)a, *(char * const *)b);` — David C. Rankin, Nov 15 '17 at 03:08
No need for first allocation. `char **wordlist = malloc(sizeof(char*)); ... wordlist[0] = strdup(words[0]);` --> `char **wordlist = NULL;` — chux - Reinstate Monica, Nov 15 '17 at 03:19

iBug · Answer 1 · 2017-11-15T04:25:32.283

2

Your comparison is wrong:

int cmpstr(const void* a, const void* b) {
    char *aa = * (char * const *)a;
    char *bb = * (char * const *)b;
//             ^
//       asterisk here!
//
    return strcmp(aa, bb);
}

a and b are pointer-to-pointers.

Or alternatively:

int cmpstr(const void* a, const void* b) {
    char * const * aa = a;
    char * const * bb = b;
    return strcmp(*aa, *bb);
}

Also, there's a potential memory leak because you didn't free() what strdup() gives. You should remove Line 3 in your given code:

wordlist[0] = strdup(words[0]);

edited Nov 15 '17 at 04:25

answered Nov 15 '17 at 03:10

iBug

35,554
7
89
134

when I change it to that it gives me the warning "initialization from incompatible pointer type [enabled by default]" and it still doesn't sort the array – McLovinIt Nov 15 '17 at 03:16
@McLovinIt Did you notice there's an asterisk after the value assign operator? – iBug Nov 15 '17 at 03:19
I got it working with the suggestion that @DavidC.Rankin posted that has now been deleted – McLovinIt Nov 15 '17 at 03:20
@McLovinIt I suppose both should work. They are essentially the same. – iBug Nov 15 '17 at 03:24
@iBug there is a difference, one is *type qualified* and one is a *pointer to const*, see [C11 Standard § (draft n1570)](http://port70.net/~nsz/c/c11/n1570.html#6.2.5) and see [Note: 29](http://port70.net/~nsz/c/c11/n1570.html#6.2.5p29) – David C. Rankin Nov 15 '17 at 03:49
@DavidC.Rankin I got you. I remember [this](http://en.cppreference.com/w/cpp/language/implicit_conversion#Qualification_conversions). – iBug Nov 15 '17 at 04:03
You are doing better than I am. I always remember there is an issue there, but have to look it up again to remember which way it goes `:)` – David C. Rankin Nov 15 '17 at 04:09
`char * const * aa = a;` --> well done! – chux - Reinstate Monica Nov 15 '17 at 04:23

qsort does nothing with cmpstr

1 Answers1