Search code examples
carraysmemory-managementstructfree

free memory of struct that has an array of other struct inside - C


I trying to use a dynamic array, when i finish using it, i try to free the memory used and i get this error.

free(): invalid next size (fast): 0x00652098 

This are the declarations of the struct variables:

struct player {
int played_time;
int min_time;
int max_time;
int errors;
int color;
};

struct Players {
struct player *array;
size_t player_number;
size_t size;
};
typedef struct Players Player_list;

This are the method used to manage the dynamic array:

void initArray(Player_list *list, size_t initialSize) {
  list->array = (struct player *) malloc(initialSize * sizeof(struct player));
  list->player_number = 0;
  list->size = initialSize;
}

void insertArray(Player_list *list, struct player element) {
  if (list->player_number == list->size) {
    list->size *= 2;
    list->array = (struct player *) realloc(list->array,
            list->size * sizeof(struct player));
  }
  list->array[list->player_number++] = element;
}

void freeArray(Player_list *list) {
  free(list->array);
  list->array = NULL;
  list->player_number = list->size = 0;
 }

int disqualified(Player_list *list, int errors) {
  int i = 0;
  for (i = 0; i < list->player_number; i++) {
    if (list->array[i].errors >= errors) {
        return 1;
    }
  }
  return 0;
  }

And here is how i use it in the main:

/**
 * When button is pressed 1 add an error to a random player
 */
void button_s_isr(void) {
    int index = rand() % (players.player_number);
    point_players->array[index].errors = point_players->array[index].errors      + 1;

}

      ... 

int main(void) {

      ...
// set up of GPIO
// get with scanf NUMBER_OF_PLAYERS and MAX_ERRORS values

int i;
for (i = 0; i < NUMBER_OF_PLAYERS; i++) {
    struct player player;
    player.color = PLAYER_COLORS[i];
    player.errors = 0;
    player.max_time = 0;
    player.min_time = 0;
    player.played_time = 0;
    insertArray(&players, player);
}

while (disqualified(&players, MAX_ERRORS) != 1) {
// wait
}
printf("\n  Se ha acabdo el juego: ");
freeArray(point_players);
return EXIT_SUCCESS;
}

I must say i am quite new to C, sorry if it is difficult to understand. What i want to do is a dynamic list of struct (players), where each player has own parameters (played_time, min_time , max_time, errors, color). And inside the main i want to have a game where i can control this parameters from each player. Any help to improve the code is appreciated.


Solution

  • the posted code:

    1. does not compile
    2. is missing definitions for PLAYER_COLORS[i], which is a bad idea to use as the number of players could exceed the available colours in the array.
    3. incorrectly calculates the size needed for the realloc()
    4. fails to check the returned values from functions like malloc() and realloc()
    5. contains a confusing (even for the OP) naming of variables and struct instances
    6. is missing the definition for num_jugadores
    7. incorrectly tries to assign a struct rather than copying the struct
    8. fails to declare an instance of struct Players

    and now, corrected code that compiles cleanly:

    caveat: not fully tested

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>  // memcpy()
    
    struct player
    {
        int played_time;
        int min_time;
        int max_time;
        int errors;
        int color;
    };
    
    struct Players
    {
        struct player *array;
        size_t player_number;
        size_t numPlayers;
    };
    
    
    //This are the method used to manage the dynamic array:
    
    void freeArray(struct Players *pArray)
    {
        free(pArray->array);
        pArray->array = NULL;
        pArray->player_number = pArray->numPlayers = 0;
    }
    
    
    void initArray( struct Players *pArray )
    {
        if( NULL == (pArray->array = malloc(sizeof(struct player)) ) )
        { // then malloc failed
            freeArray( pArray );
            exit( EXIT_FAILURE );
        }
    
        // implied else, malloc successful
    
        pArray->player_number = 0;
        pArray->numPlayers = 1;
    }
    
    
    size_t sizeof_array(size_t size)
    {
        return  size * sizeof(struct player);
    }
    
    
    void insertArray(struct Players *pArray, struct player *element)
    {
        if (pArray->player_number == pArray->numPlayers)
        { // then currently allocated memory for array of players is full
    
            struct player *temp = NULL;
            if( NULL == (temp = realloc(pArray->array, sizeof_array(pArray->numPlayers)*2) ) )
            { // then, realloc failed
                freeArray( pArray );
                exit( EXIT_FAILURE );
            }
    
            // implied else, realloc successful
    
            pArray->numPlayers *= 2;
            pArray->array = temp;
        }
    
        memcpy( &(pArray->array[pArray->player_number]), element, sizeof( struct player ) );
        pArray->player_number++;
    }
    
    //and here is how i use it in the main method:
    
    #define num_jugadores (20)
    
    int main( void )
    {
        int i;
        struct Players playerList;
    
        initArray(&playerList);
    
        for (i = 0; i < num_jugadores; i++)
        {
            struct player myPlayer;
            //player.color = PLAYER_COLORS[i];
            myPlayer.errors = 0;
            myPlayer.max_time = 0;
            myPlayer.min_time = 0;
            myPlayer.played_time = 0;
            insertArray(&playerList, &myPlayer);
        }
    
    
        //...
    
        freeArray(&playerList);
    } // end function: main