【问题标题】:Memory allocated was not freed for some reason - C由于某种原因未释放分配的内存 - C
【发布时间】:2015-08-28 19:04:59
【问题描述】:

我目前正在用 C 语言编写一个国际象棋游戏作为我的学习项目。我用 valgrind 检查了内存泄漏,发现了一些泄漏。 我检查了 valgrind 测试的日志,发现每次泄漏中总是有相同的函数链导致相同的点,我尝试调试和跟踪泄漏,不幸的是没有成功。

谁能看看下面的函数并指出可能是什么问题? 谢谢。

edit:我在 getAllMoves
中发现了漏洞 谢谢大家。

这里是源代码

//THESE ARE THE STRUCTS//
typedef struct position {
    char x;
    int  y;
    struct position * next;
}position;

typedef struct move {
    position* current_pos;
    char* promotion;
    struct move * next;
}move;

typedef struct {
    move* head;
    move* tail;
}moves;
//END OF STRUCTS//

/*
* Freeing a list of positions
*/
void freePositions(position* pos){
    if (pos != NULL){
        freePositions(pos->next);
        free(pos);
    }
}
/*
* Freeing a list of moves
*/
void freeMoves(move* move){
    if (move != NULL){
        freePositions(move->current_pos);
        freeMoves(move->next);
        if (move->promotion != NULL){
            free(move->promotion);
        }
        free(move);
    }
}

/* Checking if there was a check performed by "playing_color" player */
int isCheck(char playing_color , char curr_board[BOARD_SIZE][BOARD_SIZE]){
    moves* player_moves = getAllMoves(playing_color, curr_board);
    move * head = player_moves->head;
    while (head != NULL) {
        int x = head->current_pos->next->x - 97;
        int y = head->current_pos->next->y - 1;
        if ((curr_board[y][x] == WHITE_K && playing_color == 'B') || (curr_board[y][x] == BLACK_K && playing_color == 'W')) {
            freeMoves(player_moves->head);
            free(player_moves);
            return 1;
        }
        head = head->next;
    }
    freeMoves(player_moves->head);
    free(player_moves);
    return 0;
}

/*
* Concating a move to a list of moves
*/
moves* concatMoves(moves* moves_1, move* move2){
    if (move2==NULL || move2->current_pos == NULL){
        return moves_1;
    }
    if (moves_1->tail == NULL){
        moves_1->head = move2;
        moves_1->tail = move2;
        move2 = move2->next;
    }
    if (move2 != NULL){
        moves_1->tail->next = move2;
        moves_1->tail = move2;
        move* next_move = move2;
        while (next_move->next != NULL) {
            next_move = next_move->next;
            moves_1->tail = next_move;
        }
    }

    return moves_1;
}

/*
* create a move from (x,y) to (x+ x_offset, y+ y_offset)
*/
move* CreateMove(position * current_pos, int x_offset , int y_offset) {
    position * start_pos = calloc(1, sizeof(position));
    validate(start_pos, "CreateMove");
    position * new_pos = calloc(1,sizeof(position));
    validate(new_pos, "CreateMove");
    initPosition(new_pos, current_pos->x+x_offset, current_pos->y+y_offset);
    initPosition(start_pos, current_pos->x , current_pos->y );
    move *new_move = calloc(1,sizeof(move));
    validate(new_move, "CreateMove");
    initMove(new_move);
    new_move->current_pos = start_pos;
    new_move->current_pos->next = new_pos;
    return new_move;
}

/*
* get all possible bishop moves from the current position
*/
moves* getBishopMoves(char playing_color, char curr_board[BOARD_SIZE][BOARD_SIZE], position* current_pos){
    //flags that symbolizes we cannot move longer on the corresponding diagonal
    int moved_topRight_diag = 0; int moved_topLeft_diag = 0;
    int moved_downRight_diag = 0; int moved_downLeft_diag = 0;
    moves * bishop_moves = calloc(1, sizeof(moves));
    validate(bishop_moves, "getBishopMoves");
    for (int i = 1; i < BOARD_SIZE; i++) {
        if (moved_downLeft_diag && moved_topLeft_diag && moved_downRight_diag && moved_topRight_diag){
            break;
        }
        position next_pos = { current_pos->x + i, current_pos->y + i, NULL };
        move* new_move;
        //check if there is a legal  move from (x,y) to (x+i,y+i)
        if (!moved_topRight_diag && isValidPosition(next_pos)){
            if (IsEmpty(next_pos.x-97, next_pos.y-1, curr_board)){
                new_move = CreateMove(current_pos, i, i);
                concatMoves(bishop_moves, new_move);
            }
            else if (!isSameColor(playing_color, &next_pos, curr_board) && !IsEmpty(next_pos.x-97, next_pos.y-1, curr_board)){
                new_move = CreateMove(current_pos, i, i);
                concatMoves(bishop_moves, new_move);
                moved_topRight_diag = 1;
            }
            else {
                moved_topRight_diag = 1;
            }
        }
        //check if there is a legal non-capture move from (x,y) to (x+i,y-i)
        position next_pos1 = { current_pos->x + i, current_pos->y - i, NULL };
        if (!moved_downRight_diag && isValidPosition(next_pos1)){
            if (IsEmpty(next_pos1.x-97, next_pos1.y-1, curr_board)){
                new_move = CreateMove(current_pos, i, -i);
                concatMoves(bishop_moves, new_move);
            }
            else if (!isSameColor(playing_color, &next_pos1, curr_board) && !IsEmpty(next_pos1.x-97, next_pos1.y-1, curr_board)){
                new_move = CreateMove(current_pos, i, -i);
                concatMoves(bishop_moves, new_move);
                moved_downRight_diag = 1;
            }
            else {
                moved_downRight_diag = 1;
            }
        }
        //check if there is a legal non-capture move from (x,y) to (x-i,y-i)
        position next_pos2 = { current_pos->x - i, current_pos->y - i, NULL };
        if (!moved_downLeft_diag && isValidPosition(next_pos2)){
            if (IsEmpty(next_pos2.x-97, next_pos2.y-1, curr_board)){
                new_move = CreateMove(current_pos, -i, -i);
                concatMoves(bishop_moves, new_move);
            }
            else if (!isSameColor(playing_color, &next_pos2, curr_board) && !IsEmpty(next_pos2.x-97, next_pos2.y-1, curr_board)){
                new_move = CreateMove(current_pos, -i, -i);
                concatMoves(bishop_moves, new_move);
                moved_downLeft_diag = 1;
            }
            else {
                moved_downLeft_diag = 1;
            }
        }
        //check if there is a legal non-capture move from (x,y) to (x-i,y+i)
        position next_pos3 = { current_pos->x - i, current_pos->y + i, NULL };
        if (!moved_topLeft_diag && isValidPosition(next_pos3)){
            if (IsEmpty(next_pos3.x-97, next_pos3.y-1, curr_board)){
                new_move = CreateMove(current_pos, -i, i);
                concatMoves(bishop_moves, new_move);
            }
            else if (!isSameColor(playing_color, &next_pos3, curr_board) && !IsEmpty(next_pos3.x-97, next_pos3.y-1, curr_board)){
                new_move = CreateMove(current_pos, -i, i);
                concatMoves(bishop_moves, new_move);
                moved_topLeft_diag = 1;
            }
            else {
                moved_topLeft_diag = 1;
            }
        }

    }
    return bishop_moves;
}

/*
* get all possible queen moves from the current position , queen moves simply combine rook and bishop moves from a current position.
*/
moves * getQueenMoves(char playing_color, char curr_board[BOARD_SIZE][BOARD_SIZE], position* current_pos){
    moves * queen_moves = getBishopMoves(playing_color, curr_board, current_pos);
    moves* rook_moves = getRookMoves(playing_color, curr_board, current_pos);
    move* rooks_move_head = rook_moves->head;
    concatMoves(queen_moves, rooks_move_head);
    free(rook_moves);
    return queen_moves;
}

/*
* removing moves that end up with check the the playing color king
*/
void removeBadMoves(moves* all_moves, char playing_color, char curr_board[BOARD_SIZE][BOARD_SIZE]){
    move* curr_move = all_moves->head;

    while (curr_move != NULL){
        char temp_board[BOARD_SIZE][BOARD_SIZE];
        boardCopy(curr_board, temp_board);
        actualBoardUpdate(curr_move, temp_board, playing_color);
        if (curr_move == all_moves->head && isCheck(OppositeColor(playing_color), temp_board)) {
            all_moves->head = all_moves->head->next;
            freePositions(curr_move->current_pos);
            free(curr_move);
            curr_move = all_moves->head;
            continue;
        }
        else if (curr_move->next != NULL) {
            boardCopy(curr_board, temp_board);
            actualBoardUpdate(curr_move->next, temp_board, playing_color);
            if (isCheck(OppositeColor(playing_color), temp_board)){
                move* temp_move = curr_move->next;
                curr_move->next = curr_move->next->next;
                if (temp_move == all_moves->tail){
                    all_moves->tail = curr_move;
                }
                freePositions(temp_move->current_pos);
                free(temp_move);
                continue;
            }

        }
        curr_move = curr_move->next;
    }
}

/*
* get all possible moves from a given positiion at the board
*/
moves* getMovesFromPosition(char playing_color, char curr_board[BOARD_SIZE][BOARD_SIZE], position* current_pos){
    char square = curr_board[current_pos->y - 1][current_pos->x - 97];
    moves* poss_moves = NULL;
    if (square != EMPTY){
        if (playing_color == 'W'){
            if (square == WHITE_R){
                poss_moves = getRookMoves(playing_color, curr_board, current_pos);
            }
            else if (square == WHITE_N){
                poss_moves = getKnightMoves(playing_color, curr_board, current_pos);
            }
            else if (square == WHITE_B){
                poss_moves = getBishopMoves(playing_color, curr_board, current_pos);
            }
            else if (square == WHITE_Q){
                poss_moves = getQueenMoves(playing_color, curr_board, current_pos);
            }
            else if (square == WHITE_K){
                poss_moves = getKingMoves(playing_color, curr_board, current_pos);
            }
            else {
                poss_moves = getPawnMoves(playing_color, curr_board, current_pos);
            }
        }
        else {
            if (square == BLACK_R){
                poss_moves = getRookMoves(playing_color, curr_board, current_pos);
            }
            else if (square == BLACK_N){
                poss_moves = getKnightMoves(playing_color, curr_board, current_pos);
            }
            else if (square == BLACK_B){
                poss_moves = getBishopMoves(playing_color, curr_board, current_pos);

            }
            else if (square == BLACK_Q){
                poss_moves = getQueenMoves(playing_color, curr_board, current_pos);

            }
            else if (square == BLACK_K){
                poss_moves = getKingMoves(playing_color, curr_board, current_pos);

            }
            else {
                poss_moves = getPawnMoves(playing_color, curr_board, current_pos);
            }
        }
    }
    return poss_moves;
}

/*
* Getting all possible moves for a current player
*/
moves* getAllMoves(char playing_color, char curr_board[BOARD_SIZE][BOARD_SIZE]){
    moves* all_moves = NULL;
    position* curr_pos = NULL;
    moves* pos_moves = NULL;
    move* temp_head = NULL;
    all_moves = calloc(1, sizeof(moves));
    validate(all_moves, "getMoves");

    for (int i = 0; i < BOARD_SIZE; i++){
        for (int j = 0; j < BOARD_SIZE; j++){
            curr_pos = calloc(1, sizeof(position));
            validate(curr_pos, "getMoves");
            initPosition(curr_pos, j + 97, i + 1);
            if (isSameColor(playing_color, curr_pos, curr_board)){
                pos_moves = getMovesFromPosition(playing_color, curr_board, curr_pos);
                if (pos_moves->head != NULL){
                    temp_head = pos_moves->head;
                    concatMoves(all_moves, temp_head);
                    free(pos_moves);
                }
            }
            freePositions(curr_pos);
        }
    }
    return all_moves;
}

/*
* Getting all possible legal moves for a current player
*/
moves * getMoves(char playing_color, char curr_board[BOARD_SIZE][BOARD_SIZE]) {
    moves * all_moves = getAllMoves(playing_color, curr_board);
    removeBadMoves(all_moves, playing_color, curr_board);
    return all_moves;
}

/*
* Checking if the game has ended in a Tie
*/
int isTie(char playing_color){
    moves* possible_moves = getMoves(playing_color, board);
    if (possible_moves->head == NULL && !isCheck(OppositeColor(playing_color),board)){
        free(possible_moves);
        return 1;
    }
    freeMoves(possible_moves->head);
    free(possible_moves);
    return 0;
}

/*
* Checking if the game has ended in a Mate or a Tie
*/
int gameOver(int print_bit){
    if (isMate('W', board)){
        if (print_bit){
            printf("Mate! White player wins the game\n");
        }
        return 1;
    }
    if (isMate('B', board)){
        if (print_bit){
            printf("Mate! Black player wins the game\n");
        }
        return 1;
    }
    if (isTie('W') || isTie('B')){
        if (print_bit){
            printf("The game ends in a tie\n");
        }
        return 1;
    }
    return 0;
}

这是我执行的 valgrind 测试的日志
(所有的泄漏都包含几乎相同的函数链,所以我只发布其中的几个):

==28961== HEAP SUMMARY:  
==28961==     in use at exit: 80,480 bytes in 5,030 blocks  
==28961==   total heap usage: 124,076 allocs, 119,046 frees, 2,096,919 bytes allocated  
==28961==   
==28961== Searching for pointers to 5,030 not-freed blocks  
==28961== Checked 1,055,792 bytes 

==28961== 16 bytes in 1 blocks are definitely lost in loss record 5 of 152  
==28961==    at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)  
==28961==    by 0x40AD14: getBishopMoves (chesslogic.c:712)  
==28961==    by 0x40B745: getQueenMoves (chesslogic.c:877)  
==28961==    by 0x40C779: getMovesFromPosition (chesslogic.c:1056)  
==28961==    by 0x40C974: getAllMoves (chesslogic.c:1109)  
==28961==    by 0x40C9FF: getMoves (chesslogic.c:1126)  
==28961==    by 0x40CA3C: isTie (chesslogic.c:1136)  
==28961==    by 0x409BBD: gameOver (chesslogic.c:273)  
==28961==    by 0x40917B: SettingsMode (gameflow.c:626)  
==28961==    by 0x40CFBB: main (main.c:26)  
==28961==   
==28961== 16 bytes in 1 blocks are definitely lost in loss record 6 of 152  
==28961==    at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)  
==28961==    by 0x40BF47: getKingMoves (chesslogic.c:966)  
==28961==    by 0x40C79E: getMovesFromPosition (chesslogic.c:1059)  
==28961==    by 0x40C974: getAllMoves (chesslogic.c:1109)  
==28961==    by 0x40C9FF: getMoves (chesslogic.c:1126)  
==28961==    by 0x40CA3C: isTie (chesslogic.c:1136)  
==28961==    by 0x409BBD: gameOver (chesslogic.c:273)  
==28961==    by 0x40917B: SettingsMode (gameflow.c:626)  
==28961==    by 0x40CFBB: main (main.c:26)  
==28961==   
==28961== 16 bytes in 1 blocks are definitely lost in loss record 7 of 152  
==28961==    at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)  
==28961==    by 0x40AD14: getBishopMoves (chesslogic.c:712)  
==28961==    by 0x40B745: getQueenMoves (chesslogic.c:877)  
==28961==    by 0x40C84E: getMovesFromPosition (chesslogic.c:1077)  
==28961==    by 0x40C974: getAllMoves (chesslogic.c:1109)  
==28961==    by 0x4099C9: isCheck (chesslogic.c:220)  
==28961==    by 0x40A6C4: removeBadMoves (chesslogic.c:604)  
==28961==    by 0x40CA19: getMoves (chesslogic.c:1127)  
==28961==    by 0x40CA3C: isTie (chesslogic.c:1136)  
==28961==    by 0x409BBD: gameOver (chesslogic.c:273)  
==28961==    by 0x40917B: SettingsMode (gameflow.c:626)  
==28961==    by 0x40CFBB: main (main.c:26)  
==28961==   
==28961== 16 bytes in 1 blocks are definitely lost in loss record 8 of 152  
==28961==    at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)  
==28961==    by 0x40BF47: getKingMoves (chesslogic.c:966)  
==28961==    by 0x40C870: getMovesFromPosition (chesslogic.c:1081)  
==28961==    by 0x40C974: getAllMoves (chesslogic.c:1109)  
==28961==    by 0x4099C9: isCheck (chesslogic.c:220)  
==28961==    by 0x40A6C4: removeBadMoves (chesslogic.c:604)  
==28961==    by 0x40CA19: getMoves (chesslogic.c:1127)  
==28961==    by 0x40CA3C: isTie (chesslogic.c:1136)  
==28961==    by 0x409BBD: gameOver (chesslogic.c:273)  
==28961==    by 0x40917B: SettingsMode (gameflow.c:626)  
==28961==    by 0x40CFBB: main (main.c:26)  

【问题讨论】:

  • 不要发布不必要的截图!改为复制/粘贴文本(格式正确)。
  • FWIW,&amp;player_moves 不依赖于 malloc()free(),无论如何。
  • player_moves 中的指针 isCheck 正在被释放。 valgrind 输出实际上指向您在 getBishopMoves 中使用 calloc 进行的分配(您尚未发布的内容),因此您应该在此处查找内存泄漏。
  • 提示:由于在一个棋位上最多有 218 个合法移动(假设您根据正常规则和棋盘大小编写国际象棋),您应该分配堆栈内存用于移动生成,而不是堆分配每一步。这将获得很大的速度!
  • 大家好,感谢您的回复,我现在已经用附加功能编辑了整个问题,也许现在更清楚了。

标签: c memory-leaks valgrind allocation


【解决方案1】:

isCheck 中的 player_moves 指针正在被释放。

valgrind 输出实际上将您指向您在 getBishopMoves 中使用 calloc 进行的分配(您尚未发布的内容),因此您应该在此处查找内存泄漏的原因。

您尚未发布 moves 数据类型是什么,但如果它是一个包含您在 getBishopMoves 中为其分配内存的指针的结构,那么您应该在释放结构指针本身之前释放该指针。

类似:

free(player_moves->ptr);
free(player_moves);

【讨论】:

  • 我会尽快添加缺失的部分。 Moves 确实是一个链表,但正如您在 isCheck 的代码中看到的那样,我确实使用了 freemoves ,它释放了列表中的所有元素。此外,被指出为泄漏的 calloc 属于移动类型。这就是为什么我得出的结论是 player_moves 没有被释放。
  • 我已经编辑了问题,功能更多,日志更清晰。
【解决方案2】:

缺少代码,所以我只能猜测。另外,我不认为 stackoverflow 通常用于外包您的代码调试。

但是,我认为问题在于 player_moves 是某种链表结构,其中包含许多已分配的结构(在 getAllMoves 中分配)。 free() 只会释放 player_moves 对象,并且由于该结构中引用的指针丢失而导致泄漏。您可能应该为结构创建一个“破坏”功能,该功能会通过并删除与其链接的每个动作。也许这包含在freePositions(curr_pos) 行中,我无法从这个sn-p 中分辨出来。这取决于freePositionsconcatMoves 在做什么。

一般来说,我认为您已经陷入了这种束缚,因为您没有遵循“谁创造它就摧毁它”的原则。这在 C++ 等面向对象语言中更容易构建,但在这种情况下,我将使用函数来创建和销毁对象(移动、碎片等)并将它们传递给预先创建的对象。例如,getMovesFromPosition 函数内部分配内存并不明显。如果它接受了一个分配的对象,那么它需要在之后删除会更清楚。

【讨论】:

  • 我已经编辑了问题,功能更多,日志更清晰。
【解决方案3】:

只是对您的 freePositions() 和 freeMoves() 函数的评论。这里没有必要使用递归。以迭代方式执行此操作效率更高。

void freePositions(position* pos){
    while (pos != NULL){
        position *next = pos->next;
        free(pos);
        pos = next;
    }
}

void freeMoves(move* move){
    while (move != NULL){
        move *next = move->next;
        freePositions(move->current_pos);
        if (move->promotion != NULL){
            free(move->promotion);
        }
        free(move);
        move = next;
    }
}

【讨论】:

    猜你喜欢
    • 2011-04-11
    • 1970-01-01
    • 1970-01-01
    • 2020-10-02
    • 2012-08-19
    • 1970-01-01
    • 2012-01-28
    • 1970-01-01
    • 1970-01-01
    相关资源
    最近更新 更多