家庭作业:释放结构中的数据

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;
}