stdlib.h qsort() on char array crashes in c

Question

#include <stdio.h>
#include <stdlib.h>

int cmpfunc(const void *a, const void *b) {
   const char *ia = (const char*)a;
   const char *ib = (const char*)b;
   return *ia - *ib;
}

int is_permutation(char *s1, char *s2){
    int i, n1, n2;


    n1 = sizeof(s1)/sizeof(s1[0]);
    n2 = sizeof(s2)/sizeof(s2[0]);


    if(n1 != n2){
        return 0;
    }


    qsort(s1, n1, sizeof(s1), cmpfunc);
    qsort(s2, n2, sizeof(s2), cmpfunc);

    for (i = 0; i < n1;  i++)
       if (s1[i] != s2[i])
         return 0;

    return 1;
}

int main(){
    char s1[5] = "check";
    char s2[5] = "check";

    printf("%d", is_permutation(s1,s2));

    return 0;
}

It just crashes with no compiler errors. I've checked and the qsort crashes the program, everything else seems to work appropriately. Any help?

I compile with "gcc -g -ansi -pedantic -Wall prog.c -o prog"

If I'm not mistaken, `sizeof()` in C returns the size of the pointer, not what it points to (here, a string). — 500 - Internal Server Error, Jun 04 '18 at 14:51
`n1 = sizeof(s1)/sizeof(s1[0]);` - inside your function this is nonsensical. — AnT stands with Russia, Jun 04 '18 at 15:17
Possible duplicate of [Why isn't the size of an array parameter the same as within main?](https://stackoverflow.com/questions/1975128/why-isnt-the-size-of-an-array-parameter-the-same-as-within-main) — Bo Persson, Jun 04 '18 at 18:02

score 3 · Answer 1 · answered Jun 04 '18 at 14:52

sizeof(s1) &c. is not a function of the number of elements in the array. This is because s1 has decayed to a pointer type.

strlen can be used to get the length of a string, but you'd need to write

char s1[6] = "check";

or better still,

char s1[] = "check";

to allow space for the NUL-terminator.

score 0 · Answer 2 · edited Jun 04 '18 at 15:38

0

I've checked and the qsort crashes the program, everything else seems to work appropriately. ? No, have a run in debugger ? Try compiling with -g option and run gdb and do bt.

The problematic statement are

n1 = sizeof(s1)/sizeof(s1[0]); /* it will results in 4/1 that is 4 */
n2 = sizeof(s2)/sizeof(s2[0]);

Instead rotate loop inside s1 and find the length or use strlen() to find the length of s1 and s2 as

for(n1 = 0;s1[n1]!='\0'; n1++); /* dummy loop, at the end of loop, n1 will be length of s1 */
for(n2 = 0;s2[n2]!='\0'; n2++);

And

qsort(s1, n1, sizeof(s1[0]), cmpfunc);
qsort(s2, n2, sizeof(s2[0]), cmpfunc);

Here is the sample is_permutation() function

int is_permutation(char *s1, char *s2){
        int i, n1, n2;
        for(n1 = 0;s1[n1]!='\0'; n1++); /* dummy loop, at the end of loop, n1 will be length of s1 */
        for(n2 = 0;s2[n2]!='\0'; n2++);

        if(n1 != n2){
                return 0;
        }
        qsort(s1, n1, sizeof(s1[0]), cmpfunc);
        qsort(s2, n2, sizeof(s2[0]), cmpfunc);

        for (i = 0; i < n1;  i++)
                if (s1[i] != s2[i])
                        return 0;

        return 1;
}

Most importantly char s1[5]="check" doesn't have space for \0 char. So either make char s1[6] or char s1[]= "check"

edited Jun 04 '18 at 15:38

Persixty

8,165
2
13
35

answered Jun 04 '18 at 15:02

Achal

11,821
2
15
37

1

`strlen` will be UB since there's no space for the NUL terminator in the strings. – Bathsheba Jun 04 '18 at 15:03
yes agree @Bathsheba . for that I suggested `n1+1` and `n2+1`. Is it correct ? – Achal Jun 04 '18 at 15:04
No it isn't: `n1 = strlen(s1);` is still UB. – Bathsheba Jun 04 '18 at 15:06
Ok. I modified @Bathsheba – Achal Jun 04 '18 at 15:10
Did you reimplement `strlen` due to the issue Bathsheba mentioned? If so, this won't help as it does the same thing as `strlen`. It's the input strings that need to be fixed. – interjay Jun 04 '18 at 15:29
1

Or sizeof(s1[0]) will be fine. @Persixty – Achal Jun 04 '18 at 15:29
@achal Better still... Also you want `qsort(s1, n1, sizeof(s1[0]), cmpfunc);`. The elements of the array (i.e. string) are single characters. Of course `sizeof(s1[0])==1` is defined but it's code to make the meaning clear. – Persixty Jun 04 '18 at 15:31
I don't think you've noticed that char `s1[5]="check";` may lose the NUL terminator. There are 5 characters in `"check"` but 6 required to include the terminator. That's what Bathsheba is hinting at. – Persixty Jun 04 '18 at 15:34
1

Yes I changed @Persixty changed. Thanks. – Achal Jun 04 '18 at 15:34

stdlib.h qsort() on char array crashes in c

2 Answers2