On Tue, Sep 02, 2008 at 11:59:35AM -0400, David Lively wrote: > On Tue, 2008-09-02 at 17:54 +0200, Jim Meyering wrote: > > Daniel Veillard <veillard@xxxxxxxxxx> 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 ... Okay, commited then ! Thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list