1

I am confused as to why this function is leaking memory. It is supposed to detect checkmate in a chess recreation. It uses legal_moves(), which assigns a pointer to a head of a list of legal moves (save here) and king_under_check(), which returns a boolean value. When I run it through valgrind, I am getting a definite memory leak. Am I wrong to assume that per the valgrind output this function is where the leak originates?

My function:

    Bool is_checkmate(PlayerColor c) {

    Move *list = (Move *)(calloc(1, sizeof(Move)));
    Move *save = list;
    unsigned int num = 0;
    if(is_king_under_check(c) && !legal_moves(&save, c, &num)){
      freeList(save);
      return TRUE;
    }
    freeList(save);
    return FALSE;
    }

and I use freeList() to free the list written to in legal_moves():

    void freeList(Move *head){
    Move *temp;

    while(head != NULL){
        temp = head;    
        head = head->next_move;
        free(temp);
    }   

Here is the `valgrind` output:

    ==447557== 111,504 bytes in 4,646 blocks are definitely lost in loss record 44 of 47
    ==447557==    at 0x4837B65: calloc (vg_replace_malloc.c:752)
    ==447557==    by 0x116E55: is_checkmate (moves.c:1430)
    ==447557==    by 0x1172BF: run_mate1 (moves.c:1486)
    ==447557==    by 0x117CBD: run_mate2 (moves.c:1599)
    ==447557==    by 0x109C37: main (main.c:181)

Here is my (quite long) legal moves function to may narrow down the issue:

Bool legal_moves(Move **m, PlayerColor c, unsigned int *pcount) {
     /* Your implementation */
     Board tempBoard = CurrentBoard;
     uint64_t sq;
     Move *save = (Move *)(calloc(1, sizeof(Move)));
     Move *link = save;
     *m = save;
     Move possibleMove;
     /*Move *mNode = (Move *)(calloc(1, sizeof(Move)));*/
     unsigned int i, shift;
     for(shift = 0; shift < 64; shift++){
       /*printf("move count%d\n",*pcount);*/
       sq = 1;
       sq = sq<<shift;
       if(player[c].k & sq){
         possibleMove.from = shift;
         possibleMove.piece = KING;
         Pos *k_moves = king_move(possibleMove, c);
         unsigned int size = *(k_moves);
         unsigned int *arrPtr = k_moves;
         if(size > 0){
         for(i = 1; i < size+1; i++){
           possibleMove.to = *(arrPtr + i);
           PlayerState temp = player[c];
           PlayerState temp2 = player[1-c];
           if(is_king_under_check(c)){
             make_move(possibleMove, player[c].k, KING, c);
             if(is_king_under_check(c)){
               player[c] = temp;
               player[1-c] = temp2;
               CurrentBoard = tempBoard;
               continue;
             }
           }
           player[c] = temp;
           player[1-c] = temp2;
           CurrentBoard = tempBoard;
           make_move(possibleMove, player[c].k, KING, c);
           if(is_king_under_check(c) == TRUE){
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;

           }
           else if((board_to_pos(player[BLACK].k) == NORTH_OF(board_to_pos(player[WHITE].k))) ||
               (board_to_pos(player[BLACK].k) == EAST_OF(board_to_pos(player[WHITE].k))) ||
               (board_to_pos(player[BLACK].k) == SOUTH_OF(board_to_pos(player[WHITE].k))) ||
               (board_to_pos(player[BLACK].k) == EAST_OF(board_to_pos(player[WHITE].k))) ||
               (board_to_pos(player[BLACK].k) == SW_OF(board_to_pos(player[WHITE].k))) ||
               (board_to_pos(player[BLACK].k) == SE_OF(board_to_pos(player[WHITE].k))) ||
               (board_to_pos(player[BLACK].k) == NE_OF(board_to_pos(player[WHITE].k))) ||
               (board_to_pos(player[BLACK].k) == NW_OF(board_to_pos(player[WHITE].k)))){
               player[c] = temp;
               player[1-c] = temp2;
               CurrentBoard = tempBoard;
           }

           else if(detect_castle_move(possibleMove, c) == 1){
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
             if(is_castling_valid(c) == TRUE){
                 Move *mNode = (Move *)(calloc(1, sizeof(Move)));
                 mNode->from = shift;
                 mNode->to = *(arrPtr + i);
                 mNode->piece = KING;
                 mNode->promotion_choice = UNKNOWN;
                 mNode->next_move = NULL;

                 player[c] = temp;
                 player[1-c] = temp2;
                 CurrentBoard = tempBoard;

                 link = save;
                 while(link->next_move != NULL){
                    link = link->next_move;
                 }
                 link->next_move = mNode;
                 (*pcount)++;
                /*free(mNode);*/
              }

           }
           else{
             Move *mNode = (Move *)(calloc(1, sizeof(Move)));
             mNode->from = shift;
             mNode->to = *(arrPtr + i);
             mNode->piece = KING;
             mNode->promotion_choice = UNKNOWN;
             mNode->next_move = NULL;
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
             link = save;
             while(link->next_move != NULL){
                link = link->next_move;
             }
             link->next_move = mNode;
             (*pcount)++;
            /*free(mNode);*/
           }
         }
            free(k_moves);
         }
       }

       if(player[c].b & sq){
         possibleMove.from = shift;
         possibleMove.piece = BISHOP;
         Pos *b_moves = bishop_move(possibleMove, c);
         unsigned int size = *(b_moves);
         unsigned int *arrPtr = b_moves;
         if(size > 0){
         for(i = 1; i < size+1; i++){
           possibleMove.to = *(arrPtr + i);
           PlayerState temp = player[c];
           PlayerState temp2 = player[1-c];
           if(is_king_under_check(c)){
             make_move(possibleMove, player[c].b, BISHOP, c);
             if(is_king_under_check(c)){
               player[c] = temp;
               player[1-c] = temp2;
               CurrentBoard = tempBoard;
               continue;
             }
           }
           player[c] = temp;
           player[1-c] = temp2;
           CurrentBoard = tempBoard;
           make_move(possibleMove, player[c].b, BISHOP, c);
           if(is_king_under_check(c) == TRUE){
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
           }else{
             Move *mNode = (Move *)(calloc(1, sizeof(Move)));
             mNode->from = shift;
             mNode->to = *(arrPtr + i);
             mNode->piece = BISHOP;
             mNode->promotion_choice = UNKNOWN;
             mNode->next_move = NULL;
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
             link = save;
             while(link->next_move != NULL){
                link = link->next_move;
             }
             link->next_move = mNode;
             (*pcount)++;
            /* free(mNode);*/
           }
         }
            free(b_moves);
         }
       }
       if(player[c].p & sq){
         possibleMove.from = shift;
         possibleMove.piece = PAWN;
         Pos *p_moves = pawn_move(possibleMove, c);
         unsigned int size = *(p_moves);
         unsigned int *arrPtr = p_moves;
         if(size > 0){
         for(i = 1; i < size+1; i++){
           possibleMove.to = *(arrPtr + i);
           possibleMove.promotion_choice = UNKNOWN;
           PlayerState temp = player[c];
           PlayerState temp2 = player[1-c];

           if(is_king_under_check(c)){
             make_move(possibleMove, player[c].p, PAWN, c);
             if(is_king_under_check(c)){
               player[c] = temp;
               player[1-c] = temp2;
               CurrentBoard = tempBoard;
               continue;
             }
           }
           if(RANK_OF(possibleMove.to) == '1' || RANK_OF(possibleMove.to) == '8'){
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
             int j = 0;
             for(j = 0; j < 4; j++){
                 Move *mNode = (Move *)(calloc(1, sizeof(Move)));
                 mNode->promotion_choice = UNKNOWN;
                 mNode->from = shift;
                 mNode->to = *(arrPtr + i);
                 mNode->piece = PAWN;
                 mNode->next_move = NULL;
                 if(j == 0) mNode->promotion_choice = QUEEN;
                 if(j == 1) mNode->promotion_choice = ROOK;
                 if(j == 2) mNode->promotion_choice = BISHOP;
                 if(j == 3) mNode->promotion_choice = NIGHT;
                 player[c] = temp;
                 player[1-c] = temp2;
                 CurrentBoard = tempBoard;
                 link = save;
                 while(link->next_move != NULL){
                    link = link->next_move;
                 }
                 link->next_move = mNode;
                 (*pcount)++;
                /*free(mNode);*/
             }

           }
           player[c] = temp;
           player[1-c] = temp2;
           CurrentBoard = tempBoard;
           make_move(possibleMove, player[c].p, PAWN, c);
           if(is_king_under_check(c) == TRUE){
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
           }

           else{
             Move *mNode = (Move *)(calloc(1, sizeof(Move)));
             mNode->from = shift;
             mNode->to = *(arrPtr + i);
             mNode->piece = PAWN;
             mNode->promotion_choice = UNKNOWN;
             mNode->next_move = NULL;
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
             link = save;
             while(link->next_move != NULL){
                link = link->next_move;
             }
             link->next_move = mNode;
             (*pcount)++;
            /* free(mNode);*/
           }
         }
            free(p_moves);
         }
       }
       if(player[c].r & sq){
         possibleMove.from = shift;
         possibleMove.piece = ROOK;
         Pos *r_moves = rook_move(possibleMove, c);
         unsigned int size = *(r_moves);
         unsigned int *arrPtr = r_moves;
         if(size > 0){
         for(i = 1; i < size+1; i++){
           possibleMove.to = *(arrPtr + i);
           PlayerState temp = player[c];
           PlayerState temp2 = player[1-c];
           if(is_king_under_check(c)){
             make_move(possibleMove, player[c].r, ROOK, c);
             if(is_king_under_check(c)){
               player[c] = temp;
               player[1-c] = temp2;
               CurrentBoard = tempBoard;
               continue;
             }
           }
           player[c] = temp;
           player[1-c] = temp2;
           CurrentBoard = tempBoard;
           make_move(possibleMove, player[c].r, ROOK, c);
           if(is_king_under_check(c) == TRUE){
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
           }else{
             Move *mNode = (Move *)(calloc(1, sizeof(Move)));
             mNode->from = shift;
             mNode->to = *(arrPtr + i);
             mNode->piece = ROOK;
             mNode->promotion_choice = UNKNOWN;
             mNode->next_move = NULL;
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
             link = save;
             while(link->next_move != NULL){
                link = link->next_move;
             }
             link->next_move = mNode;
             (*pcount)++;
           }
         }
            free(r_moves);
         }

       }
       if(player[c].n & sq){
         possibleMove.from = shift;
         possibleMove.piece = NIGHT;
         Pos *n_moves = knight_move(possibleMove, c);
         unsigned int size = *(n_moves);
         unsigned int *arrPtr = n_moves;
         if(size > 0){
         for(i = 1; i < size+1; i++){
           possibleMove.to = *(arrPtr + i);
           PlayerState temp = player[c];
           PlayerState temp2 = player[1-c];
           if(is_king_under_check(c)){
             make_move(possibleMove, player[c].n, NIGHT, c);
             if(is_king_under_check(c)){
               player[c] = temp;
               player[1-c] = temp2;
               CurrentBoard = tempBoard;
               continue;
             }
           }
           player[c] = temp;
           player[1-c] = temp2;
           CurrentBoard = tempBoard;
           make_move(possibleMove, player[c].n, NIGHT, c);
           if(is_king_under_check(c) == TRUE){
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
           }else{
             Move *mNode = (Move *)(calloc(1, sizeof(Move)));
             mNode->from = shift;
             mNode->to = *(arrPtr + i);
             mNode->piece = NIGHT;
             mNode->promotion_choice = UNKNOWN;
             mNode->next_move = NULL;
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
             link = save;
             while(link->next_move != NULL){
                link = link->next_move;
             }
             link->next_move = mNode;
             (*pcount)++;
             /*free(mNode);*/
           }
          }
            free(n_moves);
         }
       }
       if(player[c].q & sq){
         possibleMove.from = shift;
         possibleMove.piece = QUEEN;
         Pos *q_moves = queen_move(possibleMove, c);
         unsigned int size = *(q_moves);
         unsigned int *arrPtr = q_moves;
         if(size > 0){
         for(i = 1; i < size+1; i++){
           possibleMove.to = *(arrPtr + i);
           PlayerState temp = player[c];
           PlayerState temp2 = player[1-c];
           if(is_king_under_check(c)){
             make_move(possibleMove, player[c].q, QUEEN, c);
             if(is_king_under_check(c)){
               player[c] = temp;
               player[1-c] = temp2;
               CurrentBoard = tempBoard;
               continue;
             }
           }
           player[c] = temp;
           player[1-c] = temp2;
           CurrentBoard = tempBoard;
           make_move(possibleMove, player[c].q, QUEEN, c);
           if(is_king_under_check(c)){
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
           }else{
             Move *mNode = (Move *)(calloc(1, sizeof(Move)));
             mNode->from = shift;
             mNode->to = *(arrPtr + i);
             mNode->piece = QUEEN;
             mNode->promotion_choice = UNKNOWN;
             mNode->next_move = NULL;
             player[c] = temp;
             player[1-c] = temp2;
             CurrentBoard = tempBoard;
             link = save;
             while(link->next_move != NULL){
                link = link->next_move;
             }
             link->next_move = mNode;
             (*pcount)++;
             /*free(mNode);*/
           }

         }
            free(q_moves);
        }
       }
     }
     if(*pcount > 0){
       return TRUE;
     }else{
       return FALSE;
     }
 }

And the associated valgrind output:

==447557== 535,008 (12,792 direct, 522,216 indirect) bytes in 533 blocks 
are definitely lost in loss record 47 of 47
==447557==    at 0x4837B65: calloc (vg_replace_malloc.c:752)
==447557==    by 0x10CA10: legal_moves (moves.c:24)
==447557==    by 0x117059: run_mate1 (moves.c:1464)
==447557==    by 0x117CBD: run_mate2 (moves.c:1599)
==447557==    by 0x109C37: main (main.c:181)
Marco Bonelli
  • 63,369
  • 21
  • 118
  • 128
d827
  • 79
  • 7
  • 1
    `legal_moves(&save, c, &num))` - if that modifies `save`, and does not free what it pointed to coming in, then the original `list` is leaked. I suspect the usefulness of `list` is questionable (i.e. the `calloc` is pointless), and in fact `save` should start as `NULL` from inception, but that sheer wag (wild-arse-guess) is based entirely on what `legal_moves` actually does with its pointer-to-pointer provided argument. – WhozCraig Apr 09 '20 at 23:52
  • You are passing `&save` to `legal_moves()`, that function probably modifies the value of `save` and you end up freeing something else. – Marco Bonelli Apr 09 '20 at 23:56
  • 2
    Yeah, `*m = save;` near the top of your `legal_moves` function instantly leaks the original pointer `save` had in your first function. Get rid of that `list` and `calloc`, initialize `save` as `NULL`, and send it in. It will probably be what you're looking for. – WhozCraig Apr 09 '20 at 23:59

1 Answers1

2

In your legal_moves() function:

Move *save = (Move *)(calloc(1, sizeof(Move)));
Move *link = save;
*m = save;

Here you just completely lost any reference to the allocated memory that was passed by reference to the function, and there's no way of freeing it (actually, you still have list in is_checkmate(), but you don't use it). If legal_moves() does the allocation on its own, then there's no point in doing it prior to calling it in is_checkmate() (or anywhere else the function is used).

You could do:

Move *save = NULL;
unsigned int num = 0;
if (is_king_under_check(c) && !legal_moves(&save, c, &num)){
    // ...
}

Also, as a rule of thumb: don't cast the result of malloc/calloc/realloc.

Marco Bonelli
  • 63,369
  • 21
  • 118
  • 128
  • I see, thank you. Though I still am getting a leak of 163,000 bytes (indirectly lost, same as the second output I posted, but less bytes) after going through the code and changing all the pointers passed into legal_moves(). Might you have any suggestions? – d827 Apr 10 '20 at 00:32
  • 1
    Follow what `valgrind` says and when you find the variable try following through the code to see how the variable gets used. It's what I did to catch the error in your question. – Marco Bonelli Apr 10 '20 at 00:40