Array of pointers to char * in c using qsort

Question

While adding string to my pointer's array, it is being overwriten by the last one. Could anyone tell me, where's my mistake?

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

int main (){
int ile = 3;
const char * slowa[ile];
for(int j = 0; j < ile; j++){
    char string[30];
    gets(string); 
    slowa[j] = string;
    printf ("%s dodalem pierwsza\n",string);
}
for (int i = 0; i < ile; i++) {
    printf ("%s numer %d\n",slowa[i],i);
}
return 0;
}

There's no qsort in your code snippet and your problem has nothing to do with sorting. Can you fix the title of your question? — Felipe Lavratti, Dec 05 '16 at 00:00
Not the issue, but: **never ever** use `gets`! It has been removed from the C standard 5 years ago and has been deprecated since 1999. Use `fgets` instead. — too honest for this site, Dec 05 '16 at 01:19
Yeah that's true @Olaf, I'm pretty sure the newest version of `gcc` warns you not to use it. — RoadRunner, Dec 05 '16 at 02:15
@RoadRunner: As gcc (and clang) does not implement the standard libary, but is - strictly speaking - a freestanding implementation, it can hardly warn. And as it is not part of the standard, your library should not declare it at all, so you get a normal "function without declaration" (or similar) warning and a linker error. — too honest for this site, Dec 05 '16 at 02:34

score 2 · Accepted Answer · answered Dec 04 '16 at 23:53

The answer is in the following two lines of code:

char string[30];
...
slowa[j] = string;

The assignment sets slowa[j] to the address of the same buffer, without making a copy. Hence, the last thing that you put in the buffer would be referenced by all elements of slowa[] array, up to position of j-1.

In order to fix this problem, make copies before storing values in slowa. You can use non-standard strdup, or use malloc+strcpy:

char string[30];
gets(string);
slowa[j] = malloc(strlen(string)+1);
strcpy(slowa[j], string);

In both cases you need to call free on all elements of slowa[] array to which you have assigned values in order to avoid memory leaks.

It is actually worse: The buffer does not exist after the first loop, so the program invokes undefined behaviour. — too honest for this site, Dec 05 '16 at 01:21

score 2 · Answer 2 · answered Dec 04 '16 at 23:58

You're always pointing to array of chars which is stack variable it's locally allocated only in scope of function, possibly each declaration of string will be on the same address as previous iteration in your loop. You could either instead of using array of chars allocate memory each loop iteration or use array and then using i.e strdup allocate memory for your new string like

slowa[j] = strdup(string) :

score 0 · Answer 3 · edited May 23 '17 at 12:09

As others have said, you need to create copies of the strings, otherwise you set the strings to the same address, and therefore they just overwrite each other.

Additionally, I think using fgets over gets is a much safer approach. This is because gets is very prone to buffer overflow, whereas with fgets, you can easily check for buffer overflow.

This is some code I wrote a while ago which is similar to what you are trying to achieve:

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

#define PTRS 3
#define STRLEN 30

int
string_cmp(const void *a, const void *b) {
    const char *str1 = *(const char**)a;
    const char *str2 = *(const char**)b;
    return strcmp(str1, str2);
}

int 
main(void) {
    char *strings[PTRS];
    char string[STRLEN];
    int str;
    size_t len, i = 0;

    while (i < PTRS) {

        printf("Enter a string: ");
        if (fgets(string, STRLEN, stdin) == NULL) {
            fprintf(stderr, "%s\n", "Error reading string");
            exit(EXIT_FAILURE);
        }

        len = strlen(string);

        if (string[len-1] == '\n') {
            string[len-1] = '\0';
        } else {
            break;
        }

        strings[i] = malloc(strlen(string)+1);
        if (strings[i] == NULL) {
            fprintf(stderr, "%s\n", "Cannot malloc string");
            exit(EXIT_FAILURE);
        }

        strcpy(strings[i], string);

        i++;
    }

    qsort(strings, i, sizeof(*strings), string_cmp);

    printf("\nSuccessfully read strings(in sorted order):\n");
    for (str = 0; str < i; str++) {
        printf("strings[%d] = %s\n", str, strings[str]);
        free(strings[str]);
        strings[str] = NULL;
    }

    return 0;
}

Array of pointers to char * in c using qsort

3 Answers3