[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] introducing <source> <name> (for logical storage pools)



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 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 ...

  Okay, commited then !

    Thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]