CODEWITHSUNDEEP
carrayspointerschar
Karol.T
Karol.T

Reputation: 65

Array of pointers to char * in c using qsort

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;
}

Upvotes: 0

Views: 122

Answers (3)

RoadRunner
RoadRunner

Reputation: 26315

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;
}

Upvotes: 0

koper89
koper89

Reputation: 655

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) :

Upvotes: 2

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726599

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.

Upvotes: 2

Related Questions