Re: [PATCH] Don't allow raneming domains to empty strings

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Thu, Jun 23, 2016 at 11:10:26PM +0200, Ján Tomko wrote:
> s/raneming/renaming/

/me hides under a rock

> On Thu, Jun 23, 2016 at 02:42:59PM -0400, John Ferlan wrote:
> > On 06/22/2016 11:48 AM, Martin Kletzander wrote:
> > > It may cause unwanted behaviour (of course, is there any wanted one for
> > > that case?) so we should rather disable the possibility of doing so.
> > >
> > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1320893
> > >
> > > Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> > > ---
> > >  src/libvirt-domain.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> > > index 508520efd6c5..89a2d7efe972 100644
> > > --- a/src/libvirt-domain.c
> > > +++ b/src/libvirt-domain.c
> > > @@ -8790,7 +8790,7 @@ virDomainRename(virDomainPtr dom,
> > >
> > >      virResetLastError();
> > >      virCheckDomainReturn(dom, -1);
> > > -    virCheckNonNullArgGoto(new_name, error);
> > > +    virCheckNonEmptyStringArgGoto(new_name, error);
> >
> > Shouldn't both be required?  EG  We don't want NULL or "" for new_name,
> > right?
>
> virCheckNonEmptyStringArgGoto also checks for NULL, so ACK with the typo
> fixed.

Yeah, I also rather checked that before sending it, although now I
realized how would you want to check for non-empty string without
checking for NULL, right? =)

Thanks, pushed now.

> Jan
>
> > The comments should at least indicate @new_name cannot be NULL or empty
> > string.
> >
> > Although it seems remoteDomainRename could pass along a NULL that it
> > doesn't seem virDomainObjListRename would be very happy to STREQ against.
> >
> > ACK as long as the NonNullArg is replaced...
> >
> > John

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list