Allocating memory to char pointers

Question

I have two issues with a function, one very hindering and the other not quite so.

I'm in the process of learning C, and the following (reduced for ease of reading) main, and findWord functions should store a randomly chosen word into 'wordToGuess' from a .txt file.

findWord stores the word into the pointer 'wordToGuess' just fine but I can't seem to extract it from the function to use it in the main. I tried making the function a 'void' and directly modifying 'wordToGuess' within the function, and making it a char* returning the word to then be used by the main, to no avail. No issues with the int* wordLength in either case, making me think the issue would stem either from a latent misunderstanding of pointers or of memory allocation.

The other less important issue is that free(wordToFind) crashes the whole thing somehow, but I'm not as fussed about that issue.

Thanks for any help.

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

#define MIN 0
#define MAX 10
#define WORDSIZE 40

char* findWord(char *wordToGuess , int *wordLength);


int main(int argc , char *argv[])
{
    int *wordLength = NULL , lengthWord = 0;
    char *wordToGuess = NULL ;

    srand(time(NULL));                                              // For rand() in findWord

    wordLength = &lengthWord;

    printf("\n1: %s\n" , findWord(wordToGuess , wordLength);        // Sole purpose of this tester printf is to verify findWord function

    return 0;
}


char* findWord(char *wordToGuess , int *wordLength)
{
    int wordNumber = 0 , i = 0;
    char word[WORDSIZE] = {0};

    FILE *wordFile = NULL;

    wordFile = fopen("words.txt" , "r");

    if(wordFile != NULL)
    {
        wordNumber = (rand() % (MAX - MIN + 1)) + MIN;                  // Random number to choose word form words.txt

        for(i = 0 ; i < wordNumber ; i++)
        {
            while(fgetc(wordFile) != '\n');                             // Move the cursor to line 'wordNumber'
        }

        fgets(word , WORDSIZE , wordFile);                              // Get word from that line
    }

    *wordLength = strlen(word);

    wordToGuess = malloc(*wordLength * sizeof(char));
    if(wordToGuess == NULL)
        exit(0);

    wordToGuess = word;

    fclose(wordFile);

   // free(wordToGuess);

    return wordToGuess;

}

Answer 1

first you allocate memory for the string

wordToGuess = malloc(*wordLength * sizeof(char));

then you immediately overwrite the pointer

wordToGuess = word;

that means:
1) wordToGuess now points to a buffer that will be invalid as soon as the function returns
2) the allocated memory is now orphaned

you probably want strcpy
you also have to allocate memory for the trailing '\0', so malloc( 1 + *wordLength )
note that sizeof(char) is 1 per definition

Answer 2

It's not a memory allocation problem. You have initialized the array with {0} and tried to assign it with the "=" operator.

Array values are NOT modifiable in C so you can't just say:

wordToGuess = word;

You can change the content of an array, but not by doing something like pointer_a = pointer_b, instead, use strcpy, in your case:

strcpy(wordToGuess, word); 

or strcpy_s, which provides the same result, but won't cause a buffer overrun if you correctly specify the size of the destination buffer.

Answer 3

Yout have two important problems here

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

#define MIN 0
#define MAX 10
#define WORDSIZE 40

char* findWord(int *wordLength);


int main(int argc , char *argv[])
{
    int lengthWord = 0;
    char *wordToGuess;

    srand(time(NULL));                                              // For rand() in findWord
    wordToGuess =  findWord(&lengthWord);
    if (wordToGuess != NULL)
    {       
        printf("\n1: %s\n" , wordToGuess);        // Sole purpose of this tester     printf is to verify findWord function
        free(wordToGuess); // you should release resources.
    }
    return 0;
}


char* findWord(int *wordLength) // You don't need to pass wordToGuess as a parameter.
{
    int wordNumber = 0 , i = 0;
    char word[WORDSIZE] = {0};
    char *wordToGuess;

    FILE *wordFile = NULL;

    wordFile = fopen("words.txt" , "r");

    if(wordFile != NULL)
    {
        wordNumber = (rand() % (MAX - MIN + 1)) + MIN;                  // Random number to choose word form words.txt

        for(i = 0 ; i < wordNumber ; i++)
        {
            while(fgetc(wordFile) != '\n');                             // Move the cursor to line     'wordNumber'
        }

        fgets(word , WORDSIZE , wordFile);                              // Get word from that line
    }

    *wordLength = strlen(word);

    //wordToGuess = malloc(*wordLength * sizeof(char)); // this is wrong
    wordToGuess = malloc(*wordLength + 1); // sizeof(char) equals 1 always
    if(wordToGuess == NULL)
        exit(0);

    // wordToGuess = word; this line is wrong, it should be
    memcpy(wordToGuess, word, *wordLength);
    wordToGuess[*wordLength] = '\0';

    fclose(wordFile);

   // free(wordToGuess);

    return wordToGuess;
}

You should always allocate space for the null terminating byte, and you should learn how pointers work, since it is clear you don't understand them from wordToGuess = word;.

Answer 4

There are many problems with your code. First and foremost is that C passes arguments by value, that means they are copied when you pass arguments to functions. So inside a function, all you have is a copy, and modifying a copy will of course not modify the original.

C doesn't have pass by reference, but it can be emulated by using pointers. In your case, you have to pass a pointer to the pointer. This can be done using the address-of operator &. Of course you have to modify the function to be able to take a pointer to a pointer as its first argument.

You are, in a way, doing it right now with the wordLength argument, but you declare it to be a pointer in the calling function, and a pointer to NULL which means that dereferencing it inside the function will lead to undefined behavior.

So somewhat correct code would be something like

char* findWord(char **wordToGuess , int *wordLength);

int main()
{
    int wordLength = 0;
    char *wordToGuess = NULL;

    printf("%s\n", findWord(&wordToGuess, &wordLength));
    // Note use of address-of operator
}

Then in the findWord function, you of course have to dereference the pointer to the string, e.g.

*wordToGuess = malloc(...);

There are also other errors in the findWord function, like you re-assigning to the pointer instead of copying to it, that will make you loose the original pointer you allocate, and make it point to a local variable and will not be valid once the function returns. Instead copy to the memory.

Another major problem in the findWord function is that you allocate one byte to little. Remember that strings contain one more character than reported by strlen, and that is the string terminator character '\0'. When you allocate memory you must add space for this character.

Answer 5

When you assign

wordToGuess = word;

you overwrite the handle to the allocated memory and effectively lose it. Even worse, you make wordToGuess point to a local array that goes out of scope as soon as you return from the function.

Your idea to allocate memory for the string that you return is good, but you have to copy the contents of the string, not the string address:

strcpy(wordToGuess, word);

You must allocate one char more than the strlen for the terminating null character, which will also be copied with strcpy.

If your system has strdup you can use it to allocate and copy in one go.