On Tue, 2008-09-02 at 17:54 +0200, Jim Meyering wrote:
Daniel Veillard <veillard@redhat.com> wrote:
diff --git a/src/storage_conf.c b/src/storage_conf.c index 2f6093b..37a2040 100644 --- a/src/storage_conf.c +++ b/src/storage_conf.c @@ -331,6 +331,8 @@ virStoragePoolDefParseDoc(virConnectPtr conn, if (ret->source.name == NULL) { /* source name defaults to pool name */ ret->source.name = strdup(ret->name); + if (ret->source.name == NULL) + virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("pool name")); } }
Hum, I'm just wondering, shouldn't we go to cleanup too on strdup error instead of continuing there ?
You're probably right. However, technically, it looks like having a NULL source.name there is tolerable, since all derefs (at least in that file) first check for non-NULL. But if a small strdup like that fails, I don't see much point in trying to continue.
If that's the intent, then it deserves a comment explaining why this failure case is different from most(all?) of the others in the vicinity.
Daniel is right. I meant to cleanup and exit (goto cleanup) in this case ... Thanks, Dave