Segmentation fault after a free()-CodePudding

I'm trying to dynamically allocate one matrix N*N and put random numbers (between 0 and h-1) in it. I also have to create a function that deallocates it. The problem is that I have to use structures and I'm not very used to them. Structure "game_t" is defined in another file and included.

game_t * newgame( int n, int m, int t){
    game_t x, *p;
    int i,j;
    p=&x;
    x.t=t;
    x.n=n;    /* structure game has n,t,h,board as keys*/
    x.h=h;
    srand(time(NULL)); 
    x.board=malloc(n*sizeof(int*));    /*Allocate*/
    if (x.board==NULL) return NULL;
    for (i=0;i<n;i  ){
              x.board[i]=malloc(n*sizeof(int));}
    for (i=0;i<n;i  ){   /*put random numbers*/
             for (j=0;i<n;j  ){
                     x.board[i][j]=rand()%h;}}
    return p;
}

void destroy(game_t *p){
    int i;
    for (i=0;i<p->n;i  ){
        free(p->board[i]);}
    free(p->board);
}

CodePudding user response：

p is a pointer to a local variable, x. Once you leave the function newgame, that variable ceases to exist, and p is now a dangling pointer. You are not allowed to access it.

The solution is straightforward: return a game_t from newgame, rather than a newgame_t *, and don’t use p:

game_t newgame(int n, int m, int t) {
    game_t x;
    int i, j;
    x.t = t;
    x.n = n;    /* structure game has n,t,h,board as keys*/
    x.h = h;
    srand(time(NULL)); 
    x.board = malloc(n * sizeof (int*));    /*Allocate*/
    if (x.board == NULL) {
        perror("newgame");
        exit(1);
    }
    for (i = 0; i < n; i  ) {
        x.board[i] = malloc(n * sizeof(int));
        if (x.board[i] == NULL) {
            perror("newgame");
            exit(1);
        }
    }
    for (i = 0; i < n; i  ) {   /*put random numbers*/
        for (j = 0; i < n; j  ) {
            x.board[i][j] = rand() % h;
        }
    }
    return x;
}

Note that this means that you can no longer return NULL if an allocation fails. I’ve amended the code to exit instead, but a different strategy might be desirable (though probably not: allocation failures are often tough to handle, and quitting — with an error message! — is an OK response).

CodePudding user response：

You return the pointer to the local variable x. Local variables disappear once you leave their scope.

game_t * newgame( int n, int m, int t){
    game_t x, *p;
    int i,j;
    p=&x;    // <<<< p points to the local variable x
    x.t=t;
    x.n=n;
    x.h=h;
    srand(time(NULL)); 
    x.board=malloc(n*sizeof(int*));
    if (x.board==NULL) return NULL;
    for (i=0;i<n;i  ){
              x.board[i]=malloc(n*sizeof(int));}
    for (i=0;i<n;i  ){
             for (j=0;i<n;j  ){
                     x.board[i][j]=rand()%h;}}
    return p;
}

You probably want this:

game_t * newgame( int n, int m, int t){
    game_t *p = malloc(sizeof (*p));   // allocate memory for a new game_t
    int i,j;
    p->t=t;
    p->n=n;
    p->h=h;
    srand(time(NULL)); 
    p->board=malloc(n*sizeof(int*));
    if (p->board==NULL) return NULL;
    for (i=0;i<n;i  ){
              p->board[i]=malloc(n*sizeof(int));}
    for (i=0;i<n;i  ){
             for (j=0;i<n;j  ){
                     p->board[i][j]=rand()%h;}}
    return p;
}

BTW: you should use meaningful variable names. For example p should be named new etc.

Bonus: call srand(time(NULL)) only once at the start of the program.