daemon/admin.c | 2 +- src/locking/lock_daemon.c | 2 +- src/locking/lock_driver_sanlock.c | 2 +- src/qemu/qemu_hotplug.c | 2 +- tests/virnettlshelpers.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-)
There were a few places in our code where the following pattern in 'if'
condition occurred:
if ((foo = bar() < 0))
do something;
This patch adjusts the conditions to the expected format:
if ((foo = bar()) < 0)
do something;
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1488192
Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
This can be especially "fun" if you try to assign multiple RNG devices to a
domain and then try to hot-unplug any of them but the first. Anyhow I used the
regex below to find these, I haven't spent much time tuning in, so in case
you can come up with a better one that yields more results like these I'm all
ears :).
[[:alnum:]_]+ = [[:print:]]+[^)]) < 0))
daemon/admin.c | 2 +-
src/locking/lock_daemon.c | 2 +-
src/locking/lock_driver_sanlock.c | 2 +-
src/qemu/qemu_hotplug.c | 2 +-
tests/virnettlshelpers.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/daemon/admin.c b/daemon/admin.c
index c5678bb99..baf310c7b 100644
--- a/daemon/admin.c
+++ b/daemon/admin.c
@@ -446,7 +446,7 @@ adminDispatchConnectGetLoggingOutputs(virNetServerPtr server ATTRIBUTE_UNUSED,
char *outputs = NULL;
int noutputs = 0;
- if ((noutputs = adminConnectGetLoggingOutputs(&outputs, args->flags) < 0)) {
+ if ((noutputs = adminConnectGetLoggingOutputs(&outputs, args->flags)) < 0) {
virNetMessageSaveError(rerr);
return -1;
}
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 6fbbf4b3d..fe3eaf903 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -1310,7 +1310,7 @@ int main(int argc, char **argv) {
}
srv = virNetDaemonGetServer(lockDaemon->dmn, "virtlockd");
- if ((rv = virLockDaemonSetupNetworkingSystemD(srv) < 0)) {
+ if ((rv = virLockDaemonSetupNetworkingSystemD(srv)) < 0) {
ret = VIR_LOCK_DAEMON_ERR_NETWORK;
goto cleanup;
}
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index b5e69c472..7513df4d7 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -313,7 +313,7 @@ virLockManagerSanlockSetupLockspace(virLockManagerSanlockDriverPtr driver)
goto error_unlink;
}
- if ((rv = virLockManagerSanlockInitLockspace(driver, &ls) < 0)) {
+ if ((rv = virLockManagerSanlockInitLockspace(driver, &ls)) < 0) {
char *err = NULL;
if (virLockManagerSanlockError(rv, &err)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9611df517..b365078ec 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5270,7 +5270,7 @@ qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
int rc;
int ret = -1;
- if ((idx = virDomainRNGFind(vm->def, rng) < 0)) {
+ if ((idx = virDomainRNGFind(vm->def, rng)) < 0) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("device not present in domain configuration"));
return -1;
diff --git a/tests/virnettlshelpers.c b/tests/virnettlshelpers.c
index b735c4e2f..f66205ef7 100644
--- a/tests/virnettlshelpers.c
+++ b/tests/virnettlshelpers.c
@@ -424,7 +424,7 @@ void testTLSWriteCertChain(const char *filename,
for (i = 0; i < ncerts; i++) {
size = sizeof(buffer);
- if ((err = gnutls_x509_crt_export(certs[i], GNUTLS_X509_FMT_PEM, buffer, &size) < 0)) {
+ if ((err = gnutls_x509_crt_export(certs[i], GNUTLS_X509_FMT_PEM, buffer, &size)) < 0) {
VIR_WARN("Failed to export certificate %s", gnutls_strerror(err));
unlink(filename);
abort();
--
2.13.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Sep 05, 2017 at 10:17:36AM +0200, Erik Skultety wrote: >There were a few places in our code where the following pattern in 'if' >condition occurred: > >if ((foo = bar() < 0)) > do something; > >This patch adjusts the conditions to the expected format: > >if ((foo = bar()) < 0) > do something; > I'd forbid this short ugly notation. Whoever is lazy to write: ... foo = bar(); if (foo < 0) ... is free to object until I send this e-mail. >Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1488192 > >Signed-off-by: Erik Skultety <eskultet@redhat.com> >--- > >This can be especially "fun" if you try to assign multiple RNG devices to a >domain and then try to hot-unplug any of them but the first. Anyhow I used the >regex below to find these, I haven't spent much time tuning in, so in case >you can come up with a better one that yields more results like these I'm all >ears :). > >[[:alnum:]_]+ = [[:print:]]+[^)]) < 0)) > I tried some other ones that would find '==' or '!=' or stuff split in multiple lines and did not find any. Reviewed-by: Martin Kletzander <mkletzan@redhat.com> -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Sep 05, 2017 at 11:02:46AM +0200, Martin Kletzander wrote: > On Tue, Sep 05, 2017 at 10:17:36AM +0200, Erik Skultety wrote: > > There were a few places in our code where the following pattern in 'if' > > condition occurred: > > > > if ((foo = bar() < 0)) > > do something; > > > > This patch adjusts the conditions to the expected format: > > > > if ((foo = bar()) < 0) > > do something; > > > > I'd forbid this short ugly notation. Whoever is lazy to write: > > ... > foo = bar(); > if (foo < 0) > ... > > is free to object until I send this e-mail. > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1488192 > > > > Signed-off-by: Erik Skultety <eskultet@redhat.com> > > --- > > > > This can be especially "fun" if you try to assign multiple RNG devices to a > > domain and then try to hot-unplug any of them but the first. Anyhow I used the > > regex below to find these, I haven't spent much time tuning in, so in case > > you can come up with a better one that yields more results like these I'm all > > ears :). > > > > [[:alnum:]_]+ = [[:print:]]+[^)]) < 0)) > > > > I tried some other ones that would find '==' or '!=' or stuff split in > multiple lines and did not find any. > > Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Pushed, thanks. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Sep 05, 2017 at 11:02:46AM +0200, Martin Kletzander wrote: > On Tue, Sep 05, 2017 at 10:17:36AM +0200, Erik Skultety wrote: > > There were a few places in our code where the following pattern in 'if' > > condition occurred: > > > > if ((foo = bar() < 0)) > > do something; > > > > This patch adjusts the conditions to the expected format: > > > > if ((foo = bar()) < 0) > > do something; > > > > I'd forbid this short ugly notation. Whoever is lazy to write: > > ... > foo = bar(); > if (foo < 0) > ... > > is free to object until I send this e-mail. > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1488192 > > > > Signed-off-by: Erik Skultety <eskultet@redhat.com> > > --- > > > > This can be especially "fun" if you try to assign multiple RNG devices to a > > domain and then try to hot-unplug any of them but the first. Anyhow I used the > > regex below to find these, I haven't spent much time tuning in, so in case > > you can come up with a better one that yields more results like these I'm all > > ears :). > > > > [[:alnum:]_]+ = [[:print:]]+[^)]) < 0)) It would be maybe worth creating a syntax-check rule for it. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.