c - 作业 : Freeing data in a struct

Question

我刚开始学习使用 valgrind 和 c，当我试图从结构中释放数据时，我有一个无效的 free() 输出。我相信这是因为数据没有从结构中正确释放。

这是我的结构:

typedef struct song_
{
    char *artist;
    char *title;
    mtime *lastPlayed;
} song;

这是试图释放它的函数:

void songDelete(song *s)
{
    //artist
    free(s->artist) ;
    //title
    free(s->title) ;
    //time
    if(NULL != s->lastPlayed)
        mtimeDelete(s->lastPlayed) ;
    //song
    free(s);
}

mtime 和 mtimeDelete 是一些用户定义的变量和方法，但我觉得它们与我的问题无关。我知道让别人帮我做作业是错误的，如果可能的话，我只是想朝着正确的方向插入。

Answer 1

不，这绝对是正确的做法。

所以，如果 valgrind 报错，可能是因为 artist、title 或 lastPlayed 中的值是实际上不是有效的指针。

这是我要检查的第一件事。

换句话说，确保你放入的东西有有效的指针。只需创建一首歌曲:

song *AchyBreakyHeart = malloc (sizeof (song));

不会填充字段(它们将被设置为任意值)。同样，

AchyBreakyHeart->artist = "Bill Ray Cyrus";

将使用字符串常量而不是堆中的有效指针填充它。

理想的做法是拥有一个类似于您提供的析构函数的“构造函数”，例如:

song *songCreate (char *artist, char *title, mtime *lastPlayed) {
    song *s = malloc (sizeof (song));
    if (s == NULL) return NULL;

    s->artist = strdup (artist);
    if (s->artist == NULL) {
        free (s);
        return NULL;
    }

    s->title = strdup (title);
    if (s->title == NULL) {
        free (s->artist);
        free (s);
        return NULL;
    }

    s->lastPlayed = mtimeDup (lastPlayed);
    if (s->lastPlayed == NULL) {
        free (s->title);
        free (s->artist);
        free (s);
        return NULL;
    }

    return s;
}

这保证了对象要么完全构建，要么根本不构建(即没有半状态)。

更好的方法是调整构造函数/析构函数对以相互结合处理 NULL，以简化这对。首先，一个稍微修改过的析构函数，唯一的变化是它可以接受 NULL 并忽略它:

void songDelete (song *s) {
    // Allow for 'songDelete (NULL)'.

    if (s != NULL) {
        free (s->artist);  // 'free (NULL)' is valid, does nothing.
        free (s->title);
        if (s->lastPlayed != NULL) {
            mtimeDelete (s->lastPlayed) ;
        }
        free (s);
    }
}

接下来，构造函数不是试图记住已分配的内容，而是最初将它们全部设置为 NULL，并在出现问题时调用析构函数:

song *songCreate (char *artist, char *title, mtime *lastPlayed) {
    // Create song, null all fields to ease destruction,
    //   then only return it if ALL allocations work.

    song *s = malloc (sizeof (song));
    if (s != NULL) {
        s->artist = s->title = s->lastPlayed = NULL;

        s->artist = strdup (artist);
        if (s->artist != NULL) {
            s->title = strdup (title);
            if (s->title != NULL) {
                s->lastPlayed = mtimeDup (lastPlayed);
                if (s->lastPlayed != NULL) {
                    return s;
                }
            }
        }
    }

    // If ANY allocation failed, destruct the song and return NULL.

    songDelete (s);
    return NULL;
}