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

1) char **set; --> char **set = NULL; 2) set[j] doesn't set word. — BLUEPIXY
– BLUEPIXY, Commented Apr 22, 2017 at 2:46
Implement @BLUEPIXY's suggestion and add strcpy(set[numberWord], word); before numberWord++;. — Bob Jarvis - Слава Україні
– Bob Jarvis - Слава Україні, Commented Apr 22, 2017 at 2:57
Thank you guys, it also worked! It worked! Thanks! But I still have a question: in a snippet of realloc manual it says: " If ptr is NULL, then the call is equivalent to malloc(size)". It confused me. lol — Tacio Costa
– Tacio Costa, Commented Apr 22, 2017 at 3:29
What do you think it was using before char **set = NULL;? (attempted access of an uninitialized variable is Undefined Behavior) Also, never assign the result of realloc to the variable itself, if realloc fails, you lose the reference to your pointer and create a memory leak. Instead use a tmp pointer, e.g. void *tmp = realloc (set, ...); if (tmp) set = tmp; — David C. Rankin
– David C. Rankin, Commented Apr 22, 2017 at 7:13

Cerulean Chelonii · Accepted Answer · 2017-04-22 03:08:16Z

1

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.

answered Apr 22, 2017 at 3:08

Cerulean Chelonii

3803 silver badges7 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Dmitri Over a year ago

realloc() will also work if the first argument is NULL, in which case it will allocate a new block. You don't need to allocate with another function first.

Cerulean Chelonii Over a year ago

My mistake, you are correct! Using set = NULL should also solve the allocation as well then.

Tacio Costa Over a year ago

It worked! Thanks! But I still have a question: in a snippet of realloc manual it says: " If ptr is NULL, then the call is equivalent to malloc(size)". It confused me. lol

Cerulean Chelonii Over a year ago

In C, if you don't specify an initial value when declaring variables, their values will be undefined, so you have to explicitly assign NULL to it.

Tacio Costa Over a year ago

Hm, thought pointers were initialized as NULL. Thank you, again. :)

David C. Rankin · Accepted Answer · 2017-04-22 08:01:42Z

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.

Wow! That's so much readable now. Thank you, mate. I've seen a lot of ways to remove the trailing '\n' but I didn't like it (until I saw yours lol). valgrind is very useful too. Liked it a lot. Thanks, again. I just did not get the "(char * const *)" in cmpstrings function. Why should be used const in that function and that snippet?
That is a more proper way to dereference a double-pointer to match the qsort compare function parameter type. Remember set is char **, you must derefernce the pointers passed. You can do that as *(char **)a or in the more proper way to reflect the const nature of the pointer as *(char * const *)a. (dereferencing a constant-pointer-to-pointer-to-char)

Collectives™ on Stack Overflow

malloc to an element of an "array" of pointers

2 Answers 2

5 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related