CODEWITHSUNDEEP
cmemory-managementdynamicmallocrealloc
user3128077
user3128077

Reputation: 43

Cannot get realloc() to work

FILE *file;
file = fopen(argv[1], "r");
char *match = argv[2];
if (file == NULL) {
    printf("File does not exist\n");
    return EXIT_FAILURE;
}
int numWords = 0, memLimit = 20;
char** words = (char**) calloc(memLimit, sizeof(char));
printf("Allocated initial array of 20 character pointers.\n");
char string[20];

while (fscanf(file, "%[a-zA-Z]%*[^a-zA-Z]", string) != EOF) {
    words[numWords] = malloc(strlen(string) + 1 * sizeof(char));
    strcpy(words[numWords], string);
    printf("Words: %s\n", words[numWords]);
    numWords++;     /*keep track of indexes, to realloc*/ 
    if (numWords == memLimit) {
        memLimit = 2 * memLimit;
        words = (char**) realloc(words, memLimit * sizeof(char*));   /*Fails here*/
        printf("Reallocated array of %d character pointers.\n", memLimit);
    }

}

Code should open and read a file containing words with punctuation, spaces etc and store in a string, but after 20 tries it throws an error, and I can't seem to get realloc() to work here, which I'm expecting to be the problem. The array is dynamically allocated 20 char pointers, at which when limit is reached, it should realloc by double. How can I get around this?

Upvotes: 1

Views: 671

Answers (4)

chux
chux

Reputation: 153338

words is a pointer to a pointer. The idea is to allocate an array of pointers.
The below is wrong as it allocates for memLimit characters rather than memLimit pointers.
This is the main issue

char** words = (char**) calloc(memLimit, sizeof(char)); // bad

So use an easy idiom: allocate memLimit groups of whatever words points to. It is easier to write, read and maintain.

char** words = calloc(memLimit, sizeof *words);

Avoid the while (scanf() != EOF) hole. Recall that various results can come from scanf() family. It returns the count of successfully scanned fields or EOF. That is typically 1 of at least 3 options. So do not test for one result you do not want, test for the one result you do want.

// while (fscanf(file, "%[a-zA-Z]%*[^a-zA-Z]", string) != EOF) {
while (fscanf(file, "%[a-zA-Z]%*[^a-zA-Z]", string) == 1) {

The above example may not every return 0, but the below easily could.

int d;
while (fscanf(file, "%d", &d) == 1) {

@Enzo Ferber rightly suggests using "%s". Further recommend to follow the above idiom and restrict input width to 1 less than the size of the buffer.

char string[20];
while (fscanf(file, "%19s", string) == 1) {

Suggest the habit of checking allocation result. 

    // better to use `size_t` rather than `int `for array sizes.
    size_t newLimit = 2u * memLimit;
    char** newptr = realloc(words, newLimit * sizeof *newptr);
    if (newptr == NULL) {
       puts("Out-of-memory");
       // Code still can use old `words` pointer of size `memLimit * sizeof *words`
       return -1;
    }
    memLimit = newLimit;
    words = newptr;
  }

Upvotes: 2

zmbq
zmbq

Reputation: 39013

Your first allocation is the problem. You allocate 20 chars and treat them as 20 char pointers. You overrun the allocated buffer and corrupt your memory.

The second allocation fails because the heap is corrupted.

Upvotes: 1

Enzo Ferber
Enzo Ferber

Reputation: 3094

Errors

  • Don't cast malloc/calloc returns. There's not need for it.
  • Your first sizeof is wrong. It should be sizeof(char*)
  • That scanf() format string. %s does the job just fine.

Code

The following code worked for me (printed one word per line):

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

int main(int argc, char *argv[])
{
    FILE *file;
    file = fopen(argv[1], "r");
    char *match = argv[2];
    if (file == NULL) {
        printf("File does not exist\n");
        return EXIT_FAILURE;
    }
    int numWords = 0, memLimit = 20;
    char **words = calloc(memLimit, sizeof(char*));
    printf("Allocated initial array of 20 character pointers.\n");
    char string[20];

    while (fscanf(file, "%s", string) != EOF) {
        words[numWords] =
            malloc(strlen(string) + 1 * sizeof(char));
        strcpy(words[numWords], string);
        printf("Words: %s\n", words[numWords]);
        numWords++; /*keep track of indexes, to realloc */
        if (numWords == memLimit) {
            memLimit = 2 * memLimit;
            words = realloc(words, memLimit * sizeof(char *));  
            printf
                ("Reallocated array of %d character pointers.\n",
                 memLimit);
        }

    }
}

Called with ./realloc realloc.c

Hope it helps.

Upvotes: 1

N8Stewart
N8Stewart

Reputation: 45

Two notes. First, you shouldn't ever cast the return value of calloc/malloc/realloc. See this for more information.

Second, as others have pointed out in comments, the first calloc statement uses sizeof(char) and not sizeof(char*) like it should.

Upvotes: 3

Related Questions