1

I am trying to create a small c program that will read a string with arbitrary size, without having any memory leaks.

According to my research, the function malloc can be used to allocate a number of bytes for whatever data we want to store.

In my program, I start by allocating space for 0 characters, and I make the pointer word point to it. Then whenever I read a single character, I make a pointer oldWord that points to word, which frees the old memory location once I allocate a larger memory location for the new character.

My research shows that the function free can be used to free an old memory location that is no longer needed. However, I am not sure where I am going wrong. Below you can see my code.

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

int main(void){
    char *word = malloc(0);

    printf("Enter name: ");
    readWord(word);
    printf("Your name is: %s\n", word);

    free(word);

    word = realloc(0);

    printf("Enter name: ");
    readWord(word);
    printf("Your name is: %s\n", word);

    free(word);

    return 0;
}

void readWord(char *word){
    int i = 0;
    char *oldWord, c = getchar();

    while(c != ' ' && c != '\n'){
        oldWord = word;
        word = realloc(word, i + 1);
        free(oldWord);
        word[i++] = c;
        c = getchar();
    }

    oldWord = word;
    word = realloc(word, i + 1);
    free(oldWord);
    word[i] = '\0';
}
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
M.S
  • 21
  • 2

2 Answers2

3

The problem as I see it here is with

   free(oldWord);

without checking the failure of realloc(). In case realloc() is success, passing the same pointer to free() causes undefined behavior.

That said, some more notes

  • a syntax like

    word = realloc(word, i + 1);
    

    is dangerous, in case realloc() fails, you'll lose the actual pointer, too. You should use a temporary pointer to hold the return value of realloc(), check for success and only then, assign it back to the original pointer, if you need.

  • In your code, c is of char type, which may not be sufficient to hold all the possible values returned by getchar(), for example, EOF. You should use an int type, that is what getchar() returns.

Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
0

There are multiple problems in your code:

  • you free the pointer you passed to realloc(). This is incorrect as realloc() will have freed the memory already if the block was moved. Otherwise the pointer is freed twice.

  • The pointer reallocated bu readWord() is never passed back to the caller.

  • Allocating a 0 sized block has unspecified behavior: it may return NULL or a valid pointer that should not be dereferenced but can be passed to free() or realloc().

  • You do not test for end of file: there is an infinite loop if the file does not have a space nor a linefeed in it, for example if the file is empty.

  • you do not have a prototype for readWord() before it is called.

Here is an improved yet simplistic version:

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

char *readWord(void);

int main(void) {
    char *word;

    printf("Enter name: ");
    word = readWord();
    if (word == NULL) {
        printf("Unexpected end of file\n");
    else
        printf("Your name is: %s\n", word);

    free(word);
    return 0;
}

char *readWord(void) {
    int c;
    size_t i = 0;
    char *word = NULL;

    while ((c = getchar()) != EOF && !isspace(c)) {
        word = realloc(word, i + 2);
        if (word == NULL)
            break;
        word[i++] = c;
        word[i] = '\0';
    }
    return word;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • `malloc(0)` is unspecified whether null pointer or not (not implementation-defined) – M.M Mar 26 '17 at 08:04