malloc to an element of an "array" of pointers

Question

there.

I'm trying to make a program that reads a number N of words (it ends when you type -) and print it sorted.

My problem is: I'm trying to use some kind of dynamic array of char*. It reallocate an element, but crashes when I try to create space on heap to a String (line 21).

Is it possible to fix? Thank you.

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

#define WORD_SIZE 11

int main()
{
        char word[WORD_SIZE];
        char **set;
        int j;
        int numberWord = 0;

        /* input */
        printf("Insert a word, or '-' to stop: ");
        fgets(word, WORD_SIZE, stdin);

        while (strcmp(word, "-\n")) {
                /* process */
                set = realloc(set, sizeof(char *) * (numberWord + 1));
                set[numberWord] = malloc(sizeof(char) * WORD_SIZE);
                numberWord++;

                /* input */
                printf("Insert a word, or '-' to stop: ");
                fgets(word, WORD_SIZE, stdin);
        }

        /* output */
        printf("\nSORTED:\n");
        for (j = 0; j <  numberWord; j++) {
                printf("%s", set[j]);
        }

        printf("\n");
        free(set);

        return 0;
}

Answer 1

In addition to initializing set and copying word, there are a number of additional somewhat subtle issues you are missing, and that can bite you. To begin lets start with a couple of non-consequential logic issues. Rarely, if ever do you actually want to store a string with the '\n' dangling off the end, just remove the newline read and included in word by fgets, e.g.

        /* remove trailing '\n' */
        size_t len = strlen (word);
        if (word[len - 1] == '\n')
            word[--len] = 0;            /* overwrite with nul-byte */

(having len also gives you the ability to allocate the exact amount of memory required to hold word, e.g. len + 1)

Which also gives you the opportunity to check for your exit condition with a simple length and character comparison at this point, e.g.

         if (len == 1 && *word == '-')   /* test for '-' */
            break;

(which also means your read loop can simply be for (;;) {...} which will not store "-" as one of your words)

As noted in my comment, never directly assign the return of the pointer being reallocated. Why? If realloc fails, set is not freed and realloc returns NULL -- which you then assign to set losing the reference to set and creating a memory leak. Instead, always use a temporary pointer, e.g.

        /* process - validate all allocations */
        void *tmp = realloc (set, sizeof *set * (numberWord + 1));
        if (!tmp) {
            fprintf (stderr, "error: virtual memory exhausted - realloc.\n");
            break;
        }
        set = tmp;

You are allocating memory for each word, but failing to free the memory you allocate for each word. You can do this in your output loop if you no longer need the memory thereafter, e.g.

    /* output */
    printf ("\nSORTED:\n");
    for (j = 0; j <  numberWord; j++) {
        printf (" %s\n", set[j]);
        free (set[j]);          /* don't forget to free word */
    }
    free(set);                  /* free pointers */

    putchar ('\n');             /* don't use printf for single char */

Above you indicate your are outputting your words in SORTED order, but nowhere do you sort your words. The simplest way to sort the words is to create a simple comparison function for the pointers in set and then pass set to qsort for sorting, e.g.

int cmpstrings (const void *a, const void *b) {
    return strcmp (*(char * const *)a, *(char * const *)b);
}

int main (void) {
    ...
    qsort (set, numberWord, sizeof *set, cmpstrings);   /* sort */

Putting all the pieces together and rearranging the logic of your request for input in a more concise manner, you could do something like the following:

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

#define WORD_SIZE 11

int cmpstrings (const void *a, const void *b) {
    return strcmp (*(char * const *)a, *(char * const *)b);
}

int main (void) {

    char word[WORD_SIZE] = "",
        **set = NULL;
    int j, numberWord = 0;

    for (;;) {

        /* input */
        printf("Insert a word, or '-' to stop: ");
        fgets(word, WORD_SIZE, stdin);

        /* remove trailing '\n' */
        size_t len = strlen (word);
        if (word[len - 1] == '\n')
            word[--len] = 0;            /* overwrite with nul-byte */

        if (len == 1 && *word == '-')   /* test for '-' */
            break;

        /* process - validate all allocations */
        void *tmp = realloc (set, sizeof *set * (numberWord + 1));
        if (!tmp) {
            fprintf (stderr, "error: virtual memory exhausted - realloc.\n");
            break;
        }
        set = tmp;

        set[numberWord] = malloc (sizeof *set[numberWord] * (len + 1));
        if (!set[numberWord]) {
            fprintf (stderr, "error: virtual memory exhausted - malloc.\n");
            break;
        }

        strcpy (set[numberWord], word);
        numberWord++;
    }

    qsort (set, numberWord, sizeof *set, cmpstrings);   /* sort */

    /* output */
    printf ("\nSORTED:\n");
    for (j = 0; j <  numberWord; j++) {
        printf (" %s\n", set[j]);
        free (set[j]);          /* don't forget to free word */
    }
    free(set);                  /* free pointers */

    putchar ('\n');             /* don't use printf for single char */

    return 0;
}

Example Use/Output

$ ./bin/dynwords
Insert a word, or '-' to stop: my
Insert a word, or '-' to stop: dog
Insert a word, or '-' to stop: has
Insert a word, or '-' to stop: fleas
Insert a word, or '-' to stop: the
Insert a word, or '-' to stop: cat
Insert a word, or '-' to stop: doesn't
Insert a word, or '-' to stop: have
Insert a word, or '-' to stop: any
Insert a word, or '-' to stop: -

SORTED:
 any
 cat
 doesn't
 dog
 fleas
 has
 have
 my
 the

In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.

It is imperative that you use a memory error checking program to insure you haven't written beyond/outside your allocated block of memory, attempted to read or base a conditional jump on an uninitialized value and finally to confirm that you have freed all the memory you have allocated.

For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it, e.g.

$ valgrind ./bin/dynwords
==10572== Memcheck, a memory error detector
==10572== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==10572== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==10572== Command: ./bin/dynwords
==10572==
Insert a word, or '-' to stop: my
Insert a word, or '-' to stop: dog
Insert a word, or '-' to stop: has
Insert a word, or '-' to stop: fleas
Insert a word, or '-' to stop: the
Insert a word, or '-' to stop: cat
Insert a word, or '-' to stop: doesn't
Insert a word, or '-' to stop: have
Insert a word, or '-' to stop: any
Insert a word, or '-' to stop: -

SORTED:
 any
 cat
 doesn't
 dog
 fleas
 has
 have
 my
 the

==10572==
==10572== HEAP SUMMARY:
==10572==     in use at exit: 0 bytes in 0 blocks
==10572==   total heap usage: 18 allocs, 18 frees, 402 bytes allocated
==10572==
==10572== All heap blocks were freed -- no leaks are possible
==10572==
==10572== For counts of detected and suppressed errors, rerun with: -v
==10572== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Always confirm you have freed all memory you have allocated and that there are no errors.

Look things over and let me know if you have any questions. There are a number of subtle changes incorporated, so if there is something you don't understand, just drop a comment below.

Answer 2

realloc requires that the memory being reallocated to have already been previously allocated (see here).

Try adding set = malloc(1); before the loop to have at least 1 byte allocated before hand.