【问题标题】:freeing memory in Circular Doubly Linked List在循环双向链表中释放内存
【发布时间】:2023-12-11 18:20:02
【问题描述】:

valgrind 告诉我,我在 XX 个块中有 XX 个字节肯定丢失了记录等等。

并且源代码在 malloc 中,但是,我认为这是因为我没有为 malloc 释放足够的内存。无论如何,我已经提供了我认为导致堆错误的代码。

我知道我没有释放 list_remove 中的内存,我很确定这是问题的唯一来源。它可能需要一些临时的转变,但我不知道这是否是唯一的问题。

list_t *list_remove(list_t *list, list_t *node) {
    list_t *oldnode = node;
    node->prev->next = node->next;
    node->next->prev = node->prev;
    if (list != oldnode) {
        free(oldnode);
        return list;
    } else {
         list_t *value = list->next == list ? NULL : list->next;
     free(oldnode);
        return value;
    }
}

void list_free(list_t *list) {
    if (list) {
       while (list_remove(list, list_last(list)) != NULL) {}
    } 
}

list last 只是给出列表的最后一个节点。

编辑:我很抱歉没有提供足够的信息,Kerrek SB,alk。这是其余的代码,您可以看到 malloc 发生在 newnode 中,我可以在其中开始创建新列表。这个结构很简单,有一个值和一个上一个,下一个:

#include <stdlib.h>
#include <string.h>
#include <stdio.h>
#include "ll.h"

struct list {
    char *value;
    struct list *next;
    struct list *prev;
};

const char *list_node_value(list_t *node) {
    return node->value;
}

list_t *list_first(list_t *list) {
    return list;
}

list_t *list_last(list_t *list) {
    return list->prev;
}

list_t *list_next(list_t *node) {
    return node->next;
}

list_t *list_previous(list_t *node) {
    return node->prev;
}

static void failed_allocation(void) {
    fprintf(stderr, "Out of memory.\n");
    abort();
}

static list_t *new_node(const char *value) {
    list_t *node = malloc(sizeof(list_t));
    if (!node) failed_allocation();
    node->value = malloc(strlen(value)+1);
    if (!node->value) failed_allocation();
    strcpy(node->value, value);
    return node;
}

list_t *list_insert_before(list_t *list, list_t *node, const char *value) {
    list_t *insert_node = new_node(value);
    insert_node->prev = node->prev;
    insert_node->next = node;
    insert_node->next->prev = insert_node;
    insert_node->prev->next = insert_node;
    if (list == node) {
        return insert_node;
    } else {
        return list;
    }
}

list_t *list_append(list_t *list, const char *value) {
    if (list) {
        (void) list_insert_before(list, list, value);
        return list;
    } else {
        list_t *node = new_node(value);
        node->prev = node->next = node;
        return node;
    }
}

list_t *list_prepend(list_t *list, const char *value) {
    if (list) {
        return list_insert_before(list, list, value);
    } else {
        list_t *node = new_node(value);
        node->prev = node->next = node;
        return node;
    }
}

list_t *list_remove(list_t *list, list_t *node) {
    list_t *oldnode = node;
    node->prev->next = node->next;
    node->next->prev = node->prev;
    if (list != oldnode) {
        free(oldnode);
        return list;
    } else {
         list_t *value = list->next == list ? NULL : list->next;
     free(oldnode);
        return value;
    }
}

void list_free(list_t *list) {
    if (list) {
       while (list_remove(list, list_last(list)) != NULL) {}
    } 
}

void list_foreach(list_t *list, void (*function)(const char*)) {
    if (list) {
        list_t *cur = list_first(list);
        do {
            function(cur->value);
            cur = cur->next;
        } while (cur != list_first(list));
    }
}

请帮忙!它仍然给我堆中的内存泄漏错误...

【问题讨论】:

  • 每个free 都需要一个malloc。由于您的代码中没有malloc,因此无法判断问题出在哪里。
  • 嘿,我在我的代码中发布了 malloc,感谢您的初步反馈。你能解释一下如何解决这个问题吗?非常感谢。

标签: c segmentation-fault malloc valgrind circular-list


【解决方案1】:

如果您担心 list_free(),我建议您在源头加强删除链以下假设,当一切完成后,您希望 *list 为 NULL(因为整个列表刚刚被删除)。

void list_free(list_t **list) 
{
    if (list && *list)
    {
        list_t* next = (*list)->next;
        while (next && (next != *list))
        {
            list_t *tmp = next;
            next = next->next;
            free(tmp);
        }

        free(*list);
        *list = NULL;
    }
}

或者类似的东西。通过传递外部列表指针的地址来调用:

list_t *list = NULL;

.. initialize and use your list...

// free the list
list_free(&list);

编辑在 OP 发布更多代码后,有几件事很明显。

  1. list_newnode() 没有设置 prevnext 的值,因此它们包含垃圾。
  2. 这里的所有其他函数都假定 (1) 正确初始化了 next 和 prev。坦率地说,我很惊讶这并没有在第二次添加开始时出现错误。

循环列表插入必须假设被插入的新节点可以是初始列表本身。看来您正在努力比它需要的更难。请记住,循环列表可以将 any 节点作为列表头,这与当前列表“头”被删除 时相比没有更好的表现。当发生这种情况时,必须有一种机制来为调用者重新建立新的列表“头”。同样的机制必须允许在删除最后一个节点时将列表头设置为 NULL。

您的代码似乎在不使用指向指针的情况下公开尝试这样做,但它们使循环链表的任务so 变得更加容易。代码中的其他注意事项:

  • 您的大多数函数似乎都试图建议调用者返回值的列表头应该是什么。相反,他们应该通过 in/out 参数强制执行
  • 任何相对于另一个插入新节点的函数都应该返回新节点。
  • list_prepend()list_append() 函数应被视为相对于列表头的核心 插入函数。其他 api(list_insert_before()list_insert_after() 等)应该完全与您在之前或之后插入的有效 existing 节点相关,正如我上面所说,返回一个指向新插入节点的指针总是。您将看到两个非基于根的插入器函数都不再传递列表头。
  • 您的大多数实用程序函数都是正确的,除了在执行取消引用之前没有检查无效指针。还有一些没有,但至少现在可以管理。

以下是围绕您的大多数功能构建的代码床。实际的节点放置例程已经完成,我尽我所能评论它们。主要测试夹具非常简单。如果这里有重大错误,我相信 SO-watchtower 会很快指出它们,但代码的重点不仅仅是修复你的;这是一个学习的东西:

#include <stdio.h>
#include <stdlib.h>
#include <memory.h>
#include <string.h>
#include <assert.h>

// node structure
typedef struct list_t {
    char *value;
    struct list_t *next;
    struct list_t *prev;
} list_t;

static void failed_allocation(void) {
    fprintf(stderr, "Out of memory.\n");
    abort();
}


// initialize a linked list header pointer. Just sets it to NULL.
void list_init(list_t** listpp)
{
    if (listpp)
        *listpp = NULL;
}

// return the value-field of a valid list node.
// otherwise return NULL if node is NULL.
const char *list_node_value(list_t *node)
{
    return (node ? node->value : NULL);
}

// return the next pointer (which may be a self-reference)
//  of a valid list_t pointer.
list_t *list_next(list_t *node)
{
    return (node ? node->next : NULL);
}

// return the previous pointer (which may be a self-reference)
//  of a valid list_t pointer.
list_t *list_previous(list_t *node)
{
    return (node ? node->prev : NULL);
}


// return the same pointer we were passed.
list_t *list_first(list_t *headp)
{
    return headp;
}

// return the previous pointer (which may be a self-reference)
//  of the given list-head pointer.
list_t *list_last(list_t *headp)
{
    return list_previous(headp);
}

// insert a new item at the end of the list, which means it
//  becomes the item previous to the head pointer. this handles
//  the case of an initially empty list, which creates the first
//  node that is self-referencing.
list_t *list_append(list_t **headpp, const char* value)
{
    if (!headpp) // error. must pass the address of a list_t ptr.
        return NULL;

    // allocate a new node.
    list_t* p = malloc(sizeof(*p));
    if (p == NULL)
        failed_allocation();

    // setup duplicate value
    p->value = (value) ? strdup(value) : NULL;

    // insert the node into the list. note that this
    //  works even when the head pointer is an initial
    //  self-referencing node.
    if (*headpp)
    {
        (*headpp)->prev->next = p;
        p->prev = (*headpp)->prev;
        p->next  = (*headpp);
        (*headpp)->prev = p;
    }
    else
    {   // no prior list. we're it. self-reference
        *headpp = p;
        p->next = p->prev = p;
    }
    return p;
}


// insert a new value into the list, returns a pointer to the
//  node allocated to hold the value. this will ALWAYS update
//  the given head pointer, since the new node is being prepended
//  to the list and by-definition becomes the new head.
list_t *list_prepend(list_t **headpp, const char* value)
{
    list_append(headpp, value);
    if (!(headpp && *headpp))
        return NULL;
    *headpp = (*headpp)->prev;
    return *headpp;
}


// insert a new node previous to the given valid node pointer.
// returns a pointer to the inserted node, or NULL on error.
list_t *list_insert_before(list_t* node, const char* value)
{
    // node *must* be a valid list_t pointer.
    if (!node)
        return NULL;
    list_prepend(&node, value);
    return node;
}


// insert a new node after the given valid node pointer.
// returns a pointer to the inserted node, or NULL on error.
list_t *list_insert_after(list_t* node, const char* value)
{
    // node *must* be a valid list_t pointer.
    if (!node)
        return NULL;
    node = node->next;
    list_prepend(&node, value);
    return node;
}


// delete a node referenced by the node pointer parameter.
//  this *can* be the root pointer, which means the root
//  must be set to the next item in the list before return.
int list_remove(list_t** headpp, list_t* node)
{
    // no list, empty list, or no node all return immediately.
    if (!(headpp && *headpp && node))
        return 1;

    // validate the node is in *this* list. it may seem odd, but
    //  we cannot just free it if the node may be in a *different*
    //  list, as it could be the other list's head-ptr.
    if (*headpp != node)
    {
        list_t *p = (*headpp)->next;
        while (p != node && p != *headpp)
            p = p->next;
        if (p == *headpp)
            return 1;
    }

    // isolate the node pointer by connecting surrounding links.
    node->next->prev = node->prev;
    node->prev->next = node->next;

    // move the head pointer if it is the same node
    if (*headpp ==  node)
        *headpp = (node != node->next) ? node->next : NULL;

    // finally we can delete the node.
    free(node->value);
    free(node);
    return 0;
}


// release the entire list. the list pointer will be reset to
//  NULL when this is finished.
void list_free(list_t **headpp)
{
    if (!(headpp && *headpp))
        return;
    while (*headpp)
        list_remove(headpp, *headpp);
}


// enumerate the list starting at the given node.
void list_foreach(list_t *listp, void (*function)(const char*))
{
    if (listp)
    {
        list_t *cur = listp;
        do {
            function(cur->value);
            cur = cur->next;
        } while (cur != listp);
    }
    printf("\n");
}

// printer callback
void print_str(const char* value)
{
    printf("%s\n", value);
}

// main entrypoint
int main(int argc, char *argv[])
{
    list_t *listp;
    list_init(&listp);

    // insert some new entries
    list_t* hello =   list_append(&listp, "Hello, Bedrock!!");
    assert(NULL != hello);
    assert(listp == hello);

    // insert Fred prior to hello. does not change the list head.
    list_t* fred = list_insert_before(hello, "Fred Flintstone");
    assert(NULL != fred);
    assert(listp == hello);
    // Hello, Bedrock!!
    // Fred Flintstone
    list_foreach(listp, print_str);

    // insert Wilma priot to Fred. does not change the list head.
    list_t* wilma = list_insert_before(fred, "Wilma Flintstone");
    assert(NULL != wilma);
    assert(list_next(wilma) == fred);
    assert(list_previous(wilma) == hello);
    // Hello, Bedrock!!
    // Wilma Flintstone
    // Fred Flintstone
    list_foreach(listp, print_str);

    list_t* barney =  list_prepend(&listp, "Barney Rubble");
    list_t* dino =    list_insert_after(wilma, "Dino");
    assert(barney != NULL);
    assert(dino != NULL);
    assert(listp == barney);
    assert(list_previous(barney) == fred);
    assert(list_next(barney) == hello);
    // Barney Rubble
    // Hello, Bedrock!!
    // Wilma Flintstone
    // Dino
    // Fred Flintstone
    list_foreach(listp, print_str);

    // remove everyone, one at a time.
    list_remove(&listp, fred);   // will not relocate the list head.
    // Barney Rubble
    // Hello, Bedrock!!
    // Wilma Flintstone
    // Dino
    list_foreach(listp, print_str);

    list_remove(&listp, hello);  // will not relocate the list head.
    // Barney Rubble
    // Wilma Flintstone
    // Dino
    list_foreach(listp, print_str);

    list_remove(&listp, barney); // will relocate the list head.
    // Wilma Flintstone
    // Dino
    list_foreach(listp, print_str);
    assert(listp == wilma);
    assert(list_next(wilma) == dino);
    assert(list_previous(listp) == dino);

    list_remove(&listp, wilma);  // will relocate the list head.
    // Dino
    list_foreach(listp, print_str);

    list_remove(&listp, dino);   // will relocate the list head;

    // generate a raft entries (a million of them)/
    char number[32];
    int i=0;
    for (;i<1000000; i++)
    {
        sprintf(number, "%d", i);
        list_append(&listp, number);
    }

    // now test freeing the entire list.
    list_free(&listp);

    return 0;
}

如果断言和转储是为了帮助验证算法的健全性而进行的洒水。结果输出应该与代码中的 cmets 匹配,是:

Hello, Bedrock!!
Fred Flintstone

Hello, Bedrock!!
Wilma Flintstone
Fred Flintstone

Barney Rubble
Hello, Bedrock!!
Wilma Flintstone
Dino
Fred Flintstone

Barney Rubble
Hello, Bedrock!!
Wilma Flintstone
Dino

Barney Rubble
Wilma Flintstone
Dino

Wilma Flintstone
Dino

Dino

最后的想法:我已经通过 valgrind 运行了这个,没有发现任何泄漏。 我很肯定它不会满足您的需求。** 大部分会(其中一半已经在那里)。

【讨论】:

  • 嗨 WhozCraig,感谢您的回复!我刚刚尝试使用您的方法,但它不起作用,可能是因为它与结构不兼容。但是,我已经编辑了我的原始帖子并提供了其余代码。请帮忙! :)
  • @user1818022 我去看看。这很可能与我认为 list_t 类型的定义方式与您的定义方式不兼容。
  • 所以我只需要设置 prev 和 next 吗?这会解决它吗?
【解决方案2】:

代码看起来没问题。

list_t 是如何定义的? list_t 是否有任何成员引用动态分配的内存?如果是这样,您可能还需要释放那些引用的内存。

【讨论】:

  • 嘿嘿!谢谢你回到我身旁。我正在编辑我的原始帖子以获取其余代码,请看一下!