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

[David Lively]

On Tue, 2008-09-02 at 17:54 +0200, Jim Meyering wrote:
> Daniel Veillard <veillard at 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