Nein, das ist definitiv der richtige Weg.
Also, wenn valgrind
beschwert, ist es wahrscheinlich, weil die Werte in artist
, title
oder lastPlayed
sind nicht wirklich gültige Zeiger.
Das ist das erste, was ich überprüfen würde.
Mit anderen Worten, stellen Sie sicher, was Sie setzen in gibt es gültige Zeiger. Erstellen Sie einfach einen Song mit:
song *AchyBreakyHeart = malloc (sizeof (song));
wird die Felder nicht ausfüllen (sie werden auf willkürliche Werte festgelegt). Ähnlich wird
AchyBreakyHeart->artist = "Bill Ray Cyrus";
wird es mit einer Zeichenfolge-Konstante anstelle eines gültigen Zeigers im Heap auffüllen.
Die ideale Sache wäre, einen „Konstruktor“ zu haben, ähnlich wie die destructor Sie zur Verfügung gestellt haben, so etwas wie:
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;
}
Dies garantiert, dass das Objekt entweder vollständig aufgebaut oder gar nicht gebaut (dh keine Halbzustände).
Noch besser wäre es, das Konstruktor/Destruktor-Paar so anzupassen, dass NULLs in Verbindung miteinander behandelt werden, um das Paar zu vereinfachen. Zunächst wird eine leicht modifizierte destructor, wobei die einzige Änderung, dass es NULL annehmen können und ignorieren:
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);
}
}
Als nächstes wird der Konstruktor, der, anstatt zu versuchen, sich daran zu erinnern, was zugeteilt wurde, stattdessen setzt sie alle auf NULL zunächst und fordert nur die destructor, wenn etwas schief geht:
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;
}
Was ist diese ungültige Ausgabe, die Sie bekommen? – emecas