[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 <mkletzan@redhat.com> --- 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
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@redhat.com> --- 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) {
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 <mkletzan@redhat.com> --- 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
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@redhat.com> --- 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
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.
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
participants (4)
-
John Ferlan -
Ján Tomko -
Martin Kletzander -
Peter Krempa