Search code examples
cpointersmemory-managementmallocdynamic-memory-allocation

Simple dynamic memory allocation bug


I'm sure you (pros) can identify the bug's' in my code, I also would appreciate any other comments on my code.

BTW, the code crashes after I run it.

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

typedef struct
{
    int x;
    int y;
}  Location;

typedef struct
{
    bool walkable;
    unsigned char walked; // number of times walked upon
} Cell;

typedef struct
{
    char name[40];  // Name of maze
    Cell **grid;    // 2D array of cells
    int rows;       // Number of rows
    int cols;       // Number of columns
    Location entrance;
} Maze;


Maze *maz_new()
{
    int i = 0;

    Maze *mazPtr = (Maze *)malloc(sizeof (Maze));

    if(!mazPtr)
    {
        puts("The memory couldn't be initilised, Press ENTER to exit");
        getchar();
        exit(-1);
    }
    else
    {
        // allocating memory for the grid
    mazPtr->grid = (Cell **) malloc((sizeof (Cell)) * (mazPtr->rows));

    for(i = 0; i < mazPtr->rows; i++)
        mazPtr->grid[i] = (Cell *) malloc((sizeof (Cell)) * (mazPtr->cols));
    }

    return mazPtr;
}


void maz_delete(Maze *maz)
{
    int i = 0;

    if (maz != NULL)
        {
            for(i = 0; i < maz->rows; i++)
                free(maz->grid[i]);

            free(maz->grid);
        }
}


int main()
{
    Maze *ptr = maz_new();
    maz_delete(ptr);

    getchar();
    return 0;
}

Thanks in advance.


Solution

  • How big is the maze supposed to be? You never initialise rows and cols.

    Your big problem, though, is that you use sizeof(Cell) when initialising grid, but it should be sizeof(Cell*).

    On my architecture, Cell is only two bytes, whereas Cell * is eight bytes, which means that you haven't allocated enough space. As you fill in this array, you are writing past the end and into some other chunk of allocated memory, at which point all bets are off. The memory allocator corrupts the contents of the array at some point, and you are left trying to free rubbish.