I can't allocate memory for this struct

Question

I've this struct and function:

typedef struct{

       char *maze_size;
       char *trophy;
       int time_max;
       char *moves;

}gameHistory;

The function receives the pointer gameHistory *gameHistoryReaded to reserve memory, but don't work.

void loadGameHistory(char fileName[], gameHistory *gameHistoryReaded, int *statusCode){
       /*  Here send error  */
       *gameHistoryReaded = (gameHistory*)malloc(LENGTH_GAME_HISTORY*sizeof(gameHistory));
       (*gameHistoryReaded).maze_size = (char *)malloc(MAX_STRING*sizeof(char));
       (*gameHistoryReaded).trophy = (char *)malloc(MAX_STRING*sizeof(char));
       (*gameHistoryReaded).moves = (char *)malloc(MAX_STRING*sizeof(char));

       FILE *file = fopen(fileName,"rb");
       char line[200];
       char *token1;
       if (file == NULL){
            printf("\nError de lectura: archivo no encontrado\n");
            *statusCode = 0; //Si se presenta algun error en la lectura del archivsetea en 0 (error)
            exit(1);
        }
        fgets(line,200,file);
        token1 = strtok(line,":");
        strcpy((*gameHistoryReaded).maze_size, token1);
        token1 = strtok(NULL,":");
        strcpy((*gameHistoryReaded).trophy, token1);
        token1 = strtok(NULL,":");
        strcpy((*gameHistoryReaded).moves, token1);
        token1 = strtok(NULL,":");
        (*gameHistoryReaded).time_max = atoi(token1);

        fclose(file);

}

int main(){

        char routegameHistory[] = "game_history.txt";
        int statusCode = 1;
        gameHistory gameHistory;

        loadGameHistory(routegameHistory, &gameHistory, &statusCode);

 }

Code::Blocks shown me this:

error: incompatible types when assigning to type 'gameHistory' from type 'struct gameHistory *'

But I think the struct are well declared, other point, I allocate memory for the int time_max?

Answer 1

gameHistory gameHistory; is an error. A typename cannot be re-used as a variable name. Let's assume you meant:

gameHistory history;

This allocates memory already. You do not need to allocate any extra memory. Simply remove the attempted malloc line from your function and all will be well.

However, further down in your function there would be a problem:

strcpy((*gameHistoryReaded).maze_size, token1);

So far you have not made maze_size point anywhere, so you cannot copy characters to it. You could either make maze_size be a char array (in which case it would not require any more allocation), or you could allocate space and point to that space:

gameHistoryReaded->maze_size = strdup(token1);

The strdup function does malloc and strcpy in one go.

The same problem occurs with trophy and moves; and finally you should check token1 != NULL each time.

Answer 2

You dereference the pointer gameHistoryRead, which yields the struct. To assign the result of malloc to the pointer itself, just dont dereference (omit the *).

Why do you pass this pointer to the function anyway? It is overwritten instantly. If you intend to return the malloc'ed struct to the caller, you need a pointer to the pointer (**gameHistoryRead). In this case, the first line would be correct, as you are assigning to the pointer itself.

Just noticed: you paas the struct already, so why did you malloc it? That makes just no sense (I suppose now you wanted to fill the passed struct, but why the malloc then?).

Just remove the malloc() of the struct completely or have a look at the code below.

However, this is bad design. It would be better to return a pointer to the malloc'ed struct, NULL on error (see code blow). Note that the caller must check the result before accessing the returned struct.

Also, you forgot to free() the malloc'ed blocks in the error-case. Remember to free() all allocated memory, not just the first struct.

You also forgot to set statuscode in case of success (if passing the struct to be filled, it was better to return the status code, not use a pointer to it).

The malloc size of the struct is wrong. sizeof(...) already yields the correct size (unless you want to return an array, but then even more is wrong).

In main(): You cannot use the same name for a variable as for a type (and you should not even think about just changing the case - this hinders readability); see main().

Sidenotes:

• Do not cast a void * as you do for the result of malloc(). See here for an explanation.
• Check the result of malloc before using. It can be NULL (no memory available). Dereferencing a NULL-pointer results in undefined behaviour (UB). This means anything can happen (it can even pass unnoticed).
• Do not use (*p).m to access a member of a struct from a pointer. Instead, use -> which has exactly been designed for this.
• If the other fields (maze_size, ...) always have a constant size, why not simply use arrays (with constant size), avoiding malloc for each? This makes cleanup much easier.

A better approach would be:

gameHistory *loadGameHistory(char fileName[])
{
    bool error = false;
    gameHistory *hist =
            malloc(sizeof(gameHistory));
    if ( hist == NULL )  return;  // fail instantly

    hist->maze_size = malloc(MAX_STRING); // sizeof(char) is 1 by standard)
    error |= hist->maze_size == NULL;

    // alloc further blocks

    // here do what you want. set error = true on error and skip rest

    // finally:
    if ( error ) {
        // out of memory: free all other blocks
        free(hist->maze_size);    // if NULL, free does nothing (standard)

        // ...

        free(hist);
        return NULL;
    }

    return hist;
}  

Answer 3

gameHistoryReaded is a pointer to gameHistory structure.

This line:

   *gameHistoryReaded = (gameHistory*)malloc(LENGTH_GAME_HISTORY*sizeof(gameHistory));

assigns the valued returned from malloc() to whatever gameHistoryReaded points to.

Probably not what you want.

Don't dereference the pointer with *.