Re: [libvirt] [PATCH] Don't allow raneming domains to empty strings
On Fri, Jun 24, 2016 at 01:04:33PM +0200, Peter Krempa wrote: On Fri, Jun 24, 2016 at 11:16:23 +0200, Martin Kletzander wrote: On Thu, Jun 23, 2016 at 11:10:26PM +0200, Ján Tomko wrote: >s/raneming/renaming/ /me hides under a rock You should probably find something deeper and/or heavier for writing this and then still pushing the patch with the typo in the summary. It's hard not to write something from the bottom of my heart and produce reply without the feelings affecting every second seven-letter word. And it's not like it's the first time this happened to me. Make me send a v2 next time, I guess. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Don't allow raneming domains to empty strings
On Fri, Jun 24, 2016 at 11:16:23 +0200, Martin Kletzander wrote: > On Thu, Jun 23, 2016 at 11:10:26PM +0200, Ján Tomko wrote: > >s/raneming/renaming/ > > /me hides under a rock You should probably find something deeper and/or heavier for writing this and then still pushing the patch with the typo in the summary. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Don't allow raneming domains to empty strings
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 --- 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 signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Don't allow raneming domains to empty strings
s/raneming/renaming/ 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 --- 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. 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Don't allow raneming domains to empty strings
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
> ---
> 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?
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
> virCheckReadOnlyGoto(dom->conn->flags, error);
>
> if (dom->conn->driver->domainRename) {
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Don't allow raneming domains to empty strings
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
---
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);
virCheckReadOnlyGoto(dom->conn->flags, error);
if (dom->conn->driver->domainRename) {
--
2.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
