1:14 p.m.
Some reported by cppcheck, maybe even Coverity, the rest
I noticed while looking at the code.
Ján Tomko (14):
tools: virshCheckpointListCollect: remove unused names
vbox: remove VBoxCGlueTerm
vbox: StartMachine: overwrite ret less often
Remove pointless assignments
Remove pointless initializations
virsh: virshStreamSourceSkip: remove unused 'off'
Do not check whether unsigned variables are negative
libxl: remove unused 'bits' from struct guest_arch
api: virDomainMemoryStats: use 'ret' variable
storage: createFileDir: remove useless 'err' variable
storage: createFileDir: use less ternary operators
vbox: reduce variable scope in vboxDumpStorageControllers
storage: storageBackendWipeLocal: reduce variable scope
Reduce scope of some variables
examples/c/admin/logging.c | 2 +-
src/libvirt-domain.c | 8 ++---
src/libvirt-host.c | 4 +--
src/libxl/libxl_capabilities.c | 1 -
src/libxl/libxl_driver.c | 2 +-
src/node_device/node_device_driver.c | 3 +-
src/openvz/openvz_driver.c | 2 +-
src/storage/storage_backend_mpath.c | 3 +-
src/storage/storage_util.c | 33 ++++++++++----------
src/test/test_driver.c | 2 +-
src/vbox/vbox_XPCOMCGlue.c | 19 ------------
src/vbox/vbox_XPCOMCGlue.h | 1 -
src/vbox/vbox_common.c | 46 +++++++++++++---------------
tests/virnumamock.c | 2 +-
tests/virrandommock.c | 2 +-
tools/virsh-checkpoint.c | 5 ---
tools/virsh-pool.c | 3 +-
tools/virsh-util.c | 3 +-
18 files changed, 53 insertions(+), 88 deletions(-)
--
2.26.2
1:14 p.m.
New subject: [libvirt PATCH 01/14] tools: virshCheckpointListCollect: remove unused names
Introduced by:
commit 689beaa47c51fb49fafa992dd468116b8f6b0782
and unused since.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-checkpoint.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c
index 33e70515ad..f3c4fe90ba 100644
--- a/tools/virsh-checkpoint.c
+++ b/tools/virsh-checkpoint.c
@@ -581,7 +581,6 @@ virshCheckpointListCollect(vshControl *ctl,
bool tree)
{
size_t i;
- char **names = NULL;
int count = -1;
virDomainCheckpointPtr *chks;
virshCheckpointListPtr checkpointlist = vshMalloc(ctl,
@@ -628,10 +627,6 @@ virshCheckpointListCollect(vshControl *ctl,
cleanup:
virshCheckpointListFree(checkpointlist);
- if (names && count > 0)
- for (i = 0; i < count; i++)
- VIR_FREE(names[i]);
- VIR_FREE(names);
return ret;
}
--
2.26.2
2:40 a.m.
New subject: [libvirt PATCH 01/14] tools: virshCheckpointListCollect: remove unused names
On Wed, Sep 23, 2020 at 08:14:50PM +0200, Ján Tomko wrote:
Introduced by:
commit 689beaa47c51fb49fafa992dd468116b8f6b0782
and unused since.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-checkpoint.c | 5 -----
1 file changed, 5 deletions(-)
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
1:14 p.m.
New subject: [libvirt PATCH 02/14] vbox: remove VBoxCGlueTerm
cppcheck reports:
src/vbox/vbox_XPCOMCGlue.c:226:21: style:
The statement 'if (hVBoxXPCOMC!=NULL) hVBoxXPCOMC=NULL' is
logically equivalent to 'hVBoxXPCOMC=NULL'.
[duplicateConditionalAssign]
It does not matter anyway because this function
is never called.
Fixes: e1506cb4eb7eab96e7ded27a23f0d8ac9697ac2a
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_XPCOMCGlue.c | 19 -------------------
src/vbox/vbox_XPCOMCGlue.h | 1 -
2 files changed, 20 deletions(-)
diff --git a/src/vbox/vbox_XPCOMCGlue.c b/src/vbox/vbox_XPCOMCGlue.c
index 3cbb679110..2936ff0edb 100644
--- a/src/vbox/vbox_XPCOMCGlue.c
+++ b/src/vbox/vbox_XPCOMCGlue.c
@@ -217,25 +217,6 @@ VBoxCGlueInit(unsigned int *version)
}
-/**
- * Terminate the C glue library.
- */
-void
-VBoxCGlueTerm(void)
-{
- if (hVBoxXPCOMC != NULL) {
-#if 0 /* VBoxRT.so doesn't like being reloaded. See @bugref{3725}. */
- dlclose(g_hVBoxXPCOMC);
-#endif
- hVBoxXPCOMC = NULL;
- }
-
- pVBoxFuncs_v2_2 = NULL;
- g_pfnGetFunctions = NULL;
-}
-
-
-
/*
* In XPCOM an array is represented by 1) a pointer to an array of pointers
* that point to the items and 2) an unsigned int representing the number of
diff --git a/src/vbox/vbox_XPCOMCGlue.h b/src/vbox/vbox_XPCOMCGlue.h
index 517b02393f..d0e579482e 100644
--- a/src/vbox/vbox_XPCOMCGlue.h
+++ b/src/vbox/vbox_XPCOMCGlue.h
@@ -36,7 +36,6 @@
extern PFNVBOXGETXPCOMCFUNCTIONS g_pfnGetFunctions;
int VBoxCGlueInit(unsigned int *version);
-void VBoxCGlueTerm(void);
typedef struct _vboxArray vboxArray;
--
2.26.2
2:41 a.m.
New subject: [libvirt PATCH 02/14] vbox: remove VBoxCGlueTerm
On Wed, Sep 23, 2020 at 08:14:51PM +0200, Ján Tomko wrote:
cppcheck reports:
src/vbox/vbox_XPCOMCGlue.c:226:21: style:
The statement 'if (hVBoxXPCOMC!=NULL) hVBoxXPCOMC=NULL' is
logically equivalent to 'hVBoxXPCOMC=NULL'.
[duplicateConditionalAssign]
It does not matter anyway because this function
is never called.
Fixes: e1506cb4eb7eab96e7ded27a23f0d8ac9697ac2a
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
1:14 p.m.
New subject: [libvirt PATCH 03/14] vbox: StartMachine: overwrite ret less often
Use goto to jump over the ret = 0 assignment
as is usual in rest of the code.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 9978741a64..99c7fbd117 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -2090,7 +2090,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine,
vboxIID *iid
int ret = -1;
if (!data->vboxObj)
- return ret;
+ return -1;
VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyTypeUtf16);
gVBoxAPI.UIMachine.GetExtraData(machine, keyTypeUtf16, &valueTypeUtf16);
@@ -2177,7 +2177,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine,
vboxIID *iid
if (NS_FAILED(rc)) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("OpenRemoteSession/LaunchVMProcess failed, domain can't
be started"));
- ret = -1;
+ goto cleanup;
} else {
PRBool completed = 0;
resultCodeUnion resultCode;
@@ -2186,19 +2186,21 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine
*machine, vboxIID *iid
rc = gVBoxAPI.UIProgress.GetCompleted(progress, &completed);
if (NS_FAILED(rc)) {
/* error */
- ret = -1;
+ goto cleanup;
}
gVBoxAPI.UIProgress.GetResultCode(progress, &resultCode);
if (RC_FAILED(resultCode)) {
/* error */
- ret = -1;
+ goto cleanup;
} else {
/* all ok set the domid */
dom->id = maxDomID + 1;
- ret = 0;
}
}
+ ret = 0;
+
+ cleanup:
VBOX_RELEASE(progress);
gVBoxAPI.UISession.Close(data->vboxSession);
--
2.26.2
2:46 a.m.
New subject: [libvirt PATCH 03/14] vbox: StartMachine: overwrite ret less often
On Wed, Sep 23, 2020 at 08:14:52PM +0200, Ján Tomko wrote:
Use goto to jump over the ret = 0 assignment
as is usual in rest of the code.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 9978741a64..99c7fbd117 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -2090,7 +2090,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine,
vboxIID *iid
int ret = -1;
if (!data->vboxObj)
- return ret;
+ return -1;
VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyTypeUtf16);
gVBoxAPI.UIMachine.GetExtraData(machine, keyTypeUtf16, &valueTypeUtf16);
@@ -2177,7 +2177,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine,
vboxIID *iid
if (NS_FAILED(rc)) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("OpenRemoteSession/LaunchVMProcess failed, domain can't
be started"));
- ret = -1;
+ goto cleanup;
} else {
PRBool completed = 0;
resultCodeUnion resultCode;
@@ -2186,19 +2186,21 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine
*machine, vboxIID *iid
rc = gVBoxAPI.UIProgress.GetCompleted(progress, &completed);
if (NS_FAILED(rc)) {
/* error */
- ret = -1;
+ goto cleanup;
This one is not semantically equivalent. But since I do not know enough about
the vbox driver I cannot tell whether it makes sense to keep the current
behaviour. It looks like it also fixes it but I cannot be sure enough.
Let's wait for a couple of days if someone knows more about that and if not,
then consider this
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
and let's see if someone experiences issues after that.
}
gVBoxAPI.UIProgress.GetResultCode(progress, &resultCode);
if (RC_FAILED(resultCode)) {
/* error */
- ret = -1;
+ goto cleanup;
} else {
/* all ok set the domid */
dom->id = maxDomID + 1;
- ret = 0;
}
}
+ ret = 0;
+
+ cleanup:
VBOX_RELEASE(progress);
gVBoxAPI.UISession.Close(data->vboxSession);
--
2.26.2
1:14 p.m.
New subject: [libvirt PATCH 04/14] Remove pointless assignments
There's no need to initialize every variable,
especially not if it's overwritten two lines later.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
examples/c/admin/logging.c | 2 +-
src/libxl/libxl_driver.c | 2 +-
src/vbox/vbox_common.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/examples/c/admin/logging.c b/examples/c/admin/logging.c
index 730ae40d9d..c183c4867d 100644
--- a/examples/c/admin/logging.c
+++ b/examples/c/admin/logging.c
@@ -29,7 +29,7 @@ int main(int argc, char **argv)
const char *set_outputs = NULL;
const char *set_filters = NULL;
- ret = c = -1;
+ ret = -1;
opterr = 0;
while ((c = getopt(argc, argv, ":hpo:f:")) > 0) {
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 161a6882f3..ea7d08cd11 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5423,7 +5423,7 @@ libxlDiskSectorSize(int domid, int devno)
return ret;
}
- path = val = NULL;
+ val = NULL;
path = g_strdup_printf("/local/domain/%d/device/vbd/%d/backend", domid,
devno);
if ((val = xs_read(handle, XBT_NULL, path, &len)) == NULL)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 99c7fbd117..2783827012 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -4618,7 +4618,7 @@ vboxSnapshotRedefine(virDomainPtr dom,
if (openSessionForMachine(data, dom->uuid, &domiid, &machine) < 0)
goto cleanup;
- rc = gVBoxAPI.UIMachine.SaveSettings(machine);
+ gVBoxAPI.UIMachine.SaveSettings(machine);
/* It may failed when the machine is not mutable. */
rc = gVBoxAPI.UIMachine.GetSettingsFilePath(machine, &settingsFilePath);
if (NS_FAILED(rc)) {
--
2.26.2
2:52 a.m.
New subject: [libvirt PATCH 04/14] Remove pointless assignments
On Wed, Sep 23, 2020 at 08:14:53PM +0200, Ján Tomko wrote:
There's no need to initialize every variable,
Automotive and some other industries would wholeheartedly disagree with you, but
that's not the point here. I would argue that initializing by default is a very
good practice in larger codebases if only for error-proofing against assumptions
in future changes. But what you are fixing are just assignments which are
mostly pointless.
especially not if it's overwritten two lines later.
That's true. Or if it is unused (e.g. the last hunk).
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
examples/c/admin/logging.c | 2 +-
src/libxl/libxl_driver.c | 2 +-
src/vbox/vbox_common.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/examples/c/admin/logging.c b/examples/c/admin/logging.c
index 730ae40d9d..c183c4867d 100644
--- a/examples/c/admin/logging.c
+++ b/examples/c/admin/logging.c
@@ -29,7 +29,7 @@ int main(int argc, char **argv)
const char *set_outputs = NULL;
const char *set_filters = NULL;
- ret = c = -1;
+ ret = -1;
opterr = 0;
while ((c = getopt(argc, argv, ":hpo:f:")) > 0) {
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 161a6882f3..ea7d08cd11 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5423,7 +5423,7 @@ libxlDiskSectorSize(int domid, int devno)
return ret;
}
- path = val = NULL;
+ val = NULL;
Why not get rid of the `val = NULL` as well?
path =
g_strdup_printf("/local/domain/%d/device/vbd/%d/backend", domid, devno);
if ((val = xs_read(handle, XBT_NULL, path, &len)) == NULL)
It's rewritten right here.
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 99c7fbd117..2783827012 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -4618,7 +4618,7 @@ vboxSnapshotRedefine(virDomainPtr dom,
if (openSessionForMachine(data, dom->uuid, &domiid, &machine) < 0)
goto cleanup;
- rc = gVBoxAPI.UIMachine.SaveSettings(machine);
+ gVBoxAPI.UIMachine.SaveSettings(machine);
Maybe this is missing a check for an error instead. Impossible to find out from
the docs that I quickly tried to look up though.
Anyway,
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
/* It may failed when the machine is not mutable. */
rc = gVBoxAPI.UIMachine.GetSettingsFilePath(machine, &settingsFilePath);
if (NS_FAILED(rc)) {
--
2.26.2
1:14 p.m.
New subject: [libvirt PATCH 05/14] Remove pointless initializations
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/test/test_driver.c | 2 +-
tests/virnumamock.c | 2 +-
tests/virrandommock.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d582c207f4..cbbfea6665 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4354,7 +4354,7 @@ testNodeGetFreePages(virConnectPtr conn G_GNUC_UNUSED,
unsigned long long *counts,
unsigned int flags)
{
- size_t i = 0, j = 0;
+ size_t i, j;
int x = 6;
virCheckFlags(0, -1);
diff --git a/tests/virnumamock.c b/tests/virnumamock.c
index 40e18e646e..d39c264a3f 100644
--- a/tests/virnumamock.c
+++ b/tests/virnumamock.c
@@ -130,7 +130,7 @@ virNumaGetPages(int node,
{
const int pages_def[] = { 4, 2 * 1024, 1 * 1024 * 1024};
const int npages_def = G_N_ELEMENTS(pages_def);
- size_t i = 0;
+ size_t i;
if (pages_size)
*pages_size = g_new0(unsigned int, npages_def);
diff --git a/tests/virrandommock.c b/tests/virrandommock.c
index 6dd15213e3..ca0520a5a3 100644
--- a/tests/virrandommock.c
+++ b/tests/virrandommock.c
@@ -69,7 +69,7 @@ int
gnutls_dh_params_generate2(gnutls_dh_params_t dparams,
unsigned int bits)
{
- int rc = 0;
+ int rc;
VIR_MOCK_REAL_INIT(gnutls_dh_params_generate2);
--
2.26.2
3:30 a.m.
New subject: [libvirt PATCH 05/14] Remove pointless initializations
On Wed, Sep 23, 2020 at 08:14:54PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/test/test_driver.c | 2 +-
tests/virnumamock.c | 2 +-
tests/virrandommock.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
As mentioned in the previous patch I do not think they are pointless in the long
run. I prefer ret, rc, rv and similar to be initialised to error/negative value
and others to their null values (numbers to zero, pointers to NULL etc.) even
though I know this is a very subjective opinion.
If you really want to push for this then it would require making
Of course the compiler can check if the value is used before initialisation.
And I hope all current compilers do that with our warning options. Except they
cannot check for that if you only pass the address of the value. I guess these
are fine, but I would not like to create a precedent.
What I think we should do instead is make an exception for defining variables
for iteration in the for loop itself. I know it's only 21 years since it was
standardised and we want to make sure all the compilers and distros can handle
it... /s
OK, I understand that defining variables in the middle of the function most of
the time clutters the code, but for 'i' and 'j' I, for one, see simply no
reason
to disallow that.
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d582c207f4..cbbfea6665 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4354,7 +4354,7 @@ testNodeGetFreePages(virConnectPtr conn G_GNUC_UNUSED,
unsigned long long *counts,
unsigned int flags)
{
- size_t i = 0, j = 0;
+ size_t i, j;
int x = 6;
virCheckFlags(0, -1);
diff --git a/tests/virnumamock.c b/tests/virnumamock.c
index 40e18e646e..d39c264a3f 100644
--- a/tests/virnumamock.c
+++ b/tests/virnumamock.c
@@ -130,7 +130,7 @@ virNumaGetPages(int node,
{
const int pages_def[] = { 4, 2 * 1024, 1 * 1024 * 1024};
const int npages_def = G_N_ELEMENTS(pages_def);
- size_t i = 0;
+ size_t i;
if (pages_size)
*pages_size = g_new0(unsigned int, npages_def);
diff --git a/tests/virrandommock.c b/tests/virrandommock.c
index 6dd15213e3..ca0520a5a3 100644
--- a/tests/virrandommock.c
+++ b/tests/virrandommock.c
@@ -69,7 +69,7 @@ int
gnutls_dh_params_generate2(gnutls_dh_params_t dparams,
unsigned int bits)
{
- int rc = 0;
+ int rc;
VIR_MOCK_REAL_INIT(gnutls_dh_params_generate2);
--
2.26.2
3:37 a.m.
New subject: [libvirt PATCH 05/14] Remove pointless initializations
On Thu, Sep 24, 2020 at 10:30:21AM +0200, Martin Kletzander wrote:
On Wed, Sep 23, 2020 at 08:14:54PM +0200, Ján Tomko wrote:
> Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
> ---
> src/test/test_driver.c | 2 +-
> tests/virnumamock.c | 2 +-
> tests/virrandommock.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
As mentioned in the previous patch I do not think they are pointless in the long
run. I prefer ret, rc, rv and similar to be initialised to error/negative value
and others to their null values (numbers to zero, pointers to NULL etc.) even
though I know this is a very subjective opinion.
Yeah, I tend to agree. From a defensive coding POV it is preferrable to
initialize every single variable at time of declaration to a known value.
While compilers and coverity can check for use of uninitialized variables,
I don't think their checks have ever been perfect, because new versions of
compilers tend to discover things periodically.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
1:14 p.m.
New subject: [libvirt PATCH 06/14] virsh: virshStreamSourceSkip: remove unused 'off'
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-util.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index b8b0f98066..af7ed55348 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -173,9 +173,8 @@ virshStreamSourceSkip(virStreamPtr st G_GNUC_UNUSED,
{
virshStreamCallbackDataPtr cbData = opaque;
int fd = cbData->fd;
- off_t cur;
- if ((cur = lseek(fd, offset, SEEK_CUR)) == (off_t) -1)
+ if (lseek(fd, offset, SEEK_CUR) == (off_t) -1)
return -1;
return 0;
--
2.26.2
3:31 a.m.
New subject: [libvirt PATCH 06/14] virsh: virshStreamSourceSkip: remove unused 'off'
On Wed, Sep 23, 2020 at 08:14:55PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-util.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index b8b0f98066..af7ed55348 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -173,9 +173,8 @@ virshStreamSourceSkip(virStreamPtr st G_GNUC_UNUSED,
{
virshStreamCallbackDataPtr cbData = opaque;
int fd = cbData->fd;
- off_t cur;
- if ((cur = lseek(fd, offset, SEEK_CUR)) == (off_t) -1)
+ if (lseek(fd, offset, SEEK_CUR) == (off_t) -1)
return -1;
return 0;
--
2.26.2
1:14 p.m.
New subject: [libvirt PATCH 07/14] Do not check whether unsigned variables are negative
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/openvz/openvz_driver.c | 2 +-
src/vbox/vbox_common.c | 9 ++++-----
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 71e270ea09..74d87b1085 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1215,7 +1215,7 @@ static int openvzDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
return -1;
- if (nvcpus <= 0) {
+ if (nvcpus == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Number of vCPUs should be >= 1"));
goto cleanup;
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 2783827012..b9b72e2e02 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -2111,7 +2111,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine,
vboxIID *iid
VBOX_UTF16_TO_UTF8(valueDisplayUtf16, &valueDisplayUtf8);
VBOX_UTF16_FREE(valueDisplayUtf16);
- if (strlen(valueDisplayUtf8) <= 0)
+ if (strlen(valueDisplayUtf8) == 0)
VBOX_UTF8_FREE(valueDisplayUtf8);
}
@@ -2990,7 +2990,7 @@ vboxHostDeviceGetXMLDesc(vboxDriverPtr data, virDomainDefPtr def,
IMachine *mach
gVBoxAPI.UArray.vboxArrayGet(&deviceFilters, USBCommon,
gVBoxAPI.UArray.handleUSBGetDeviceFilters(USBCommon));
- if (deviceFilters.count <= 0)
+ if (deviceFilters.count == 0)
goto release_filters;
/* check if the filters are active and then only
@@ -3612,9 +3612,8 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data,
IMachine *machine
gVBoxAPI.UArray.vboxArrayGet(&sharedFolders, machine,
gVBoxAPI.UArray.handleMachineGetSharedFolders(machine));
- if (sharedFolders.count <= 0) {
- if (sharedFolders.count == 0)
- ret = 0;
+ if (sharedFolders.count == 0) {
+ ret = 0;
goto cleanup;
}
--
2.26.2
3:33 a.m.
New subject: [libvirt PATCH 07/14] Do not check whether unsigned variables are negative
On Wed, Sep 23, 2020 at 08:14:56PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/openvz/openvz_driver.c | 2 +-
src/vbox/vbox_common.c | 9 ++++-----
2 files changed, 5 insertions(+), 6 deletions(-)
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
1:14 p.m.
New subject: [libvirt PATCH 08/14] libxl: remove unused 'bits' from struct guest_arch
It was made pointless by:
commit c25c18f71bdc43a1305be4ad1a2ca91b25cf13f3
Convert capabilities / domain_conf to use virArch
and unused by:
commit 8db1f2d228bb2f27a729a873dcdb81ce3c7c38fd
Fix libxl driver for virArch changes
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libxl/libxl_capabilities.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 8284ea3d53..e5b144ea1d 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -52,7 +52,6 @@ enum libxlHwcapVersion {
struct guest_arch {
virArch arch;
- int bits;
int hvm;
int pvh;
int pae;
--
2.26.2
3:34 a.m.
New subject: [libvirt PATCH 08/14] libxl: remove unused 'bits' from struct guest_arch
On Wed, Sep 23, 2020 at 08:14:57PM +0200, Ján Tomko wrote:
It was made pointless by:
commit c25c18f71bdc43a1305be4ad1a2ca91b25cf13f3
Convert capabilities / domain_conf to use virArch
and unused by:
commit 8db1f2d228bb2f27a729a873dcdb81ce3c7c38fd
Fix libxl driver for virArch changes
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/libxl/libxl_capabilities.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 8284ea3d53..e5b144ea1d 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -52,7 +52,6 @@ enum libxlHwcapVersion {
struct guest_arch {
virArch arch;
- int bits;
int hvm;
int pvh;
int pae;
--
2.26.2
1:14 p.m.
New subject: [libvirt PATCH 09/14] api: virDomainMemoryStats: use 'ret' variable
Instead of 'nr_stats_ret'. Also reduce its scope.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libvirt-domain.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index cde86c77e8..415482a526 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -5741,7 +5741,6 @@ virDomainMemoryStats(virDomainPtr dom, virDomainMemoryStatPtr
stats,
unsigned int nr_stats, unsigned int flags)
{
virConnectPtr conn;
- unsigned long nr_stats_ret = 0;
VIR_DOMAIN_DEBUG(dom, "stats=%p, nr_stats=%u, flags=0x%x",
stats, nr_stats, flags);
@@ -5758,11 +5757,10 @@ virDomainMemoryStats(virDomainPtr dom, virDomainMemoryStatPtr
stats,
conn = dom->conn;
if (conn->driver->domainMemoryStats) {
- nr_stats_ret = conn->driver->domainMemoryStats(dom, stats, nr_stats,
- flags);
- if (nr_stats_ret == -1)
+ int ret = conn->driver->domainMemoryStats(dom, stats, nr_stats, flags);
+ if (ret == -1)
goto error;
- return nr_stats_ret;
+ return ret;
}
virReportUnsupportedError();
--
2.26.2
3:36 a.m.
New subject: [libvirt PATCH 09/14] api: virDomainMemoryStats: use 'ret' variable
On Wed, Sep 23, 2020 at 08:14:58PM +0200, Ján Tomko wrote:
Instead of 'nr_stats_ret'. Also reduce its scope.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/libvirt-domain.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index cde86c77e8..415482a526 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -5741,7 +5741,6 @@ virDomainMemoryStats(virDomainPtr dom, virDomainMemoryStatPtr
stats,
unsigned int nr_stats, unsigned int flags)
{
virConnectPtr conn;
- unsigned long nr_stats_ret = 0;
VIR_DOMAIN_DEBUG(dom, "stats=%p, nr_stats=%u, flags=0x%x",
stats, nr_stats, flags);
@@ -5758,11 +5757,10 @@ virDomainMemoryStats(virDomainPtr dom, virDomainMemoryStatPtr
stats,
conn = dom->conn;
if (conn->driver->domainMemoryStats) {
- nr_stats_ret = conn->driver->domainMemoryStats(dom, stats, nr_stats,
- flags);
- if (nr_stats_ret == -1)
+ int ret = conn->driver->domainMemoryStats(dom, stats, nr_stats, flags);
+ if (ret == -1)
goto error;
- return nr_stats_ret;
+ return ret;
}
virReportUnsupportedError();
--
2.26.2
1:14 p.m.
New subject: [libvirt PATCH 10/14] storage: createFileDir: remove useless 'err' variable
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/storage/storage_util.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 9171cb084f..93c24ab6bc 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1997,7 +1997,6 @@ createFileDir(virStoragePoolObjPtr pool,
unsigned int flags)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- int err;
virCheckFlags(0, -1);
@@ -2015,14 +2014,14 @@ createFileDir(virStoragePoolObjPtr pool,
}
- if ((err = virDirCreate(vol->target.path,
- (vol->target.perms->mode == (mode_t)-1 ?
- VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
- vol->target.perms->mode),
- vol->target.perms->uid,
- vol->target.perms->gid,
- (def->type == VIR_STORAGE_POOL_NETFS
- ? VIR_DIR_CREATE_AS_UID : 0))) < 0) {
+ if (virDirCreate(vol->target.path,
+ (vol->target.perms->mode == (mode_t)-1 ?
+ VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
+ vol->target.perms->mode),
+ vol->target.perms->uid,
+ vol->target.perms->gid,
+ (def->type == VIR_STORAGE_POOL_NETFS
+ ? VIR_DIR_CREATE_AS_UID : 0)) < 0) {
return -1;
}
--
2.26.2
3:36 a.m.
New subject: [libvirt PATCH 10/14] storage: createFileDir: remove useless 'err' variable
On Wed, Sep 23, 2020 at 08:14:59PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/storage/storage_util.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 9171cb084f..93c24ab6bc 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1997,7 +1997,6 @@ createFileDir(virStoragePoolObjPtr pool,
unsigned int flags)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- int err;
virCheckFlags(0, -1);
@@ -2015,14 +2014,14 @@ createFileDir(virStoragePoolObjPtr pool,
}
- if ((err = virDirCreate(vol->target.path,
- (vol->target.perms->mode == (mode_t)-1 ?
- VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
- vol->target.perms->mode),
- vol->target.perms->uid,
- vol->target.perms->gid,
- (def->type == VIR_STORAGE_POOL_NETFS
- ? VIR_DIR_CREATE_AS_UID : 0))) < 0) {
+ if (virDirCreate(vol->target.path,
+ (vol->target.perms->mode == (mode_t)-1 ?
+ VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
+ vol->target.perms->mode),
+ vol->target.perms->uid,
+ vol->target.perms->gid,
+ (def->type == VIR_STORAGE_POOL_NETFS
+ ? VIR_DIR_CREATE_AS_UID : 0)) < 0) {
return -1;
}
--
2.26.2
1:15 p.m.
New subject: [libvirt PATCH 11/14] storage: createFileDir: use less ternary operators
Introduce separate variables and if conditions
with spaces around them to make the function call
easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/storage/storage_util.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 93c24ab6bc..49ecbc5344 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1997,6 +1997,8 @@ createFileDir(virStoragePoolObjPtr pool,
unsigned int flags)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+ mode_t permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
+ unsigned int flags = 0;
virCheckFlags(0, -1);
@@ -2013,15 +2015,17 @@ createFileDir(virStoragePoolObjPtr pool,
return -1;
}
+ if (vol->target.perms->mode != (mode_t)-1)
+ permmode = vol->target.perms->mode;
+
+ if (def->type == VIR_STORAGE_POOL_NETFS)
+ flags |= VIR_DIR_CREATE_AS_UID;
if (virDirCreate(vol->target.path,
- (vol->target.perms->mode == (mode_t)-1 ?
- VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
- vol->target.perms->mode),
+ permmode,
vol->target.perms->uid,
vol->target.perms->gid,
- (def->type == VIR_STORAGE_POOL_NETFS
- ? VIR_DIR_CREATE_AS_UID : 0)) < 0) {
+ flags) < 0) {
return -1;
}
--
2.26.2
1:30 p.m.
New subject: [libvirt PATCH 11/14] storage: createFileDir: use less ternary operators
On a Wednesday in 2020, Ján Tomko wrote:
Introduce separate variables and if conditions
with spaces around them to make the function call
easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/storage/storage_util.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 93c24ab6bc..49ecbc5344 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1997,6 +1997,8 @@ createFileDir(virStoragePoolObjPtr pool,
unsigned int flags)
This function already has flags ^
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+ mode_t permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
+ unsigned int flags = 0;
Consider
s/flags/createflags/
squashed in from here on
Jano
3:40 a.m.
New subject: [libvirt PATCH 11/14] storage: createFileDir: use less ternary operators
On Wed, Sep 23, 2020 at 08:15:00PM +0200, Ján Tomko wrote:
Introduce separate variables and if conditions
with spaces around them to make the function call
easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/storage/storage_util.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 93c24ab6bc..49ecbc5344 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1997,6 +1997,8 @@ createFileDir(virStoragePoolObjPtr pool,
unsigned int flags)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+ mode_t permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
+ unsigned int flags = 0;
virCheckFlags(0, -1);
@@ -2013,15 +2015,17 @@ createFileDir(virStoragePoolObjPtr pool,
return -1;
}
+ if (vol->target.perms->mode != (mode_t)-1)
+ permmode = vol->target.perms->mode;
+
It would be even less repetitive if you did:
mode_t permmode = vol->target.perms->mode;
if (permmode == -1)
permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
But
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
with your squash, of course.
+ if (def->type == VIR_STORAGE_POOL_NETFS)
+ flags |= VIR_DIR_CREATE_AS_UID;
if (virDirCreate(vol->target.path,
- (vol->target.perms->mode == (mode_t)-1 ?
- VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
- vol->target.perms->mode),
+ permmode,
vol->target.perms->uid,
vol->target.perms->gid,
- (def->type == VIR_STORAGE_POOL_NETFS
- ? VIR_DIR_CREATE_AS_UID : 0)) < 0) {
+ flags) < 0) {
return -1;
}
--
2.26.2
1:15 p.m.
New subject: [libvirt PATCH 12/14] vbox: reduce variable scope in vboxDumpStorageControllers
Most of the variables were reinitialized on every iteration.
Also remove the pointless initialization of 'i'.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index b9b72e2e02..6a517ff96c 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3079,23 +3079,18 @@ static int
vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine)
{
vboxArray storageControllers = VBOX_ARRAY_INITIALIZER;
- IStorageController *controller = NULL;
- PRUint32 storageBus = StorageBus_Null;
- PRUint32 controllerType = StorageControllerType_Null;
- virDomainControllerDefPtr cont = NULL;
- size_t i = 0;
- int model = -1, ret = -1;
- virDomainControllerType type = VIR_DOMAIN_CONTROLLER_TYPE_LAST;
+ int ret = -1;
+ size_t i;
gVBoxAPI.UArray.vboxArrayGet(&storageControllers, machine,
gVBoxAPI.UArray.handleMachineGetStorageControllers(machine));
for (i = 0; i < storageControllers.count; i++) {
- controller = storageControllers.items[i];
- storageBus = StorageBus_Null;
- controllerType = StorageControllerType_Null;
- type = VIR_DOMAIN_CONTROLLER_TYPE_LAST;
- model = -1;
+ IStorageController *controller = storageControllers.items[i];
+ PRUint32 storageBus = StorageBus_Null;
+ PRUint32 controllerType = StorageControllerType_Null;
+ virDomainControllerType type = VIR_DOMAIN_CONTROLLER_TYPE_LAST;
+ int model = -1;
if (!controller)
continue;
@@ -3133,8 +3128,6 @@ vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine)
case StorageControllerType_IntelAhci:
case StorageControllerType_I82078:
case StorageControllerType_Null:
- model = -1;
-
break;
}
@@ -3165,6 +3158,8 @@ vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine)
}
if (type != VIR_DOMAIN_CONTROLLER_TYPE_LAST) {
+ virDomainControllerDefPtr cont;
+
cont = virDomainDefAddController(def, type, -1, model);
if (!cont) {
virReportError(VIR_ERR_INTERNAL_ERROR,
--
2.26.2
3:42 a.m.
New subject: [libvirt PATCH 12/14] vbox: reduce variable scope in vboxDumpStorageControllers
On Wed, Sep 23, 2020 at 08:15:01PM +0200, Ján Tomko wrote:
Most of the variables were reinitialized on every iteration.
Also remove the pointless initialization of 'i'.
Same reply about the 'i' (👁), for the rest:
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/vbox/vbox_common.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index b9b72e2e02..6a517ff96c 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3079,23 +3079,18 @@ static int
vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine)
{
vboxArray storageControllers = VBOX_ARRAY_INITIALIZER;
- IStorageController *controller = NULL;
- PRUint32 storageBus = StorageBus_Null;
- PRUint32 controllerType = StorageControllerType_Null;
- virDomainControllerDefPtr cont = NULL;
- size_t i = 0;
- int model = -1, ret = -1;
- virDomainControllerType type = VIR_DOMAIN_CONTROLLER_TYPE_LAST;
+ int ret = -1;
+ size_t i;
gVBoxAPI.UArray.vboxArrayGet(&storageControllers, machine,
gVBoxAPI.UArray.handleMachineGetStorageControllers(machine));
for (i = 0; i < storageControllers.count; i++) {
- controller = storageControllers.items[i];
- storageBus = StorageBus_Null;
- controllerType = StorageControllerType_Null;
- type = VIR_DOMAIN_CONTROLLER_TYPE_LAST;
- model = -1;
+ IStorageController *controller = storageControllers.items[i];
+ PRUint32 storageBus = StorageBus_Null;
+ PRUint32 controllerType = StorageControllerType_Null;
+ virDomainControllerType type = VIR_DOMAIN_CONTROLLER_TYPE_LAST;
+ int model = -1;
if (!controller)
continue;
@@ -3133,8 +3128,6 @@ vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine)
case StorageControllerType_IntelAhci:
case StorageControllerType_I82078:
case StorageControllerType_Null:
- model = -1;
-
break;
}
@@ -3165,6 +3158,8 @@ vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine)
}
if (type != VIR_DOMAIN_CONTROLLER_TYPE_LAST) {
+ virDomainControllerDefPtr cont;
+
cont = virDomainDefAddController(def, type, -1, model);
if (!cont) {
virReportError(VIR_ERR_INTERNAL_ERROR,
--
2.26.2
1:15 p.m.
New subject: [libvirt PATCH 13/14] storage: storageBackendWipeLocal: reduce variable scope
Also use MIN instead of open-coding it.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/storage/storage_util.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 49ecbc5344..23632fc884 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2526,10 +2526,8 @@ storageBackendWipeLocal(const char *path,
size_t writebuf_length,
bool zero_end)
{
- int written = 0;
unsigned long long remaining = 0;
off_t size;
- size_t write_size = 0;
g_autofree char *writebuf = NULL;
if (VIR_ALLOC_N(writebuf, writebuf_length) < 0)
@@ -2557,9 +2555,9 @@ storageBackendWipeLocal(const char *path,
remaining = wipe_len;
while (remaining > 0) {
+ size_t write_size = MIN(writebuf_length, remaining);
+ int written = safewrite(fd, writebuf, write_size);
- write_size = (writebuf_length < remaining) ? writebuf_length : remaining;
- written = safewrite(fd, writebuf, write_size);
if (written < 0) {
virReportSystemError(errno,
_("Failed to write %zu bytes to "
--
2.26.2
3:42 a.m.
New subject: [libvirt PATCH 13/14] storage: storageBackendWipeLocal: reduce variable scope
On Wed, Sep 23, 2020 at 08:15:02PM +0200, Ján Tomko wrote:
Also use MIN instead of open-coding it.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/storage/storage_util.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 49ecbc5344..23632fc884 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2526,10 +2526,8 @@ storageBackendWipeLocal(const char *path,
size_t writebuf_length,
bool zero_end)
{
- int written = 0;
unsigned long long remaining = 0;
off_t size;
- size_t write_size = 0;
g_autofree char *writebuf = NULL;
if (VIR_ALLOC_N(writebuf, writebuf_length) < 0)
@@ -2557,9 +2555,9 @@ storageBackendWipeLocal(const char *path,
remaining = wipe_len;
while (remaining > 0) {
+ size_t write_size = MIN(writebuf_length, remaining);
+ int written = safewrite(fd, writebuf, write_size);
- write_size = (writebuf_length < remaining) ? writebuf_length : remaining;
- written = safewrite(fd, writebuf, write_size);
if (written < 0) {
virReportSystemError(errno,
_("Failed to write %zu bytes to "
--
2.26.2
1:15 p.m.
New subject: [libvirt PATCH 14/14] Reduce scope of some variables
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libvirt-host.c | 4 +---
src/node_device/node_device_driver.c | 3 ++-
src/storage/storage_backend_mpath.c | 3 +--
src/storage/storage_util.c | 6 ++----
tools/virsh-pool.c | 3 ++-
5 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/src/libvirt-host.c b/src/libvirt-host.c
index 07d13585f4..58622d415d 100644
--- a/src/libvirt-host.c
+++ b/src/libvirt-host.c
@@ -1352,8 +1352,6 @@ virConnectSetKeepAlive(virConnectPtr conn,
int interval,
unsigned int count)
{
- int ret = -1;
-
VIR_DEBUG("conn=%p, interval=%d, count=%u", conn, interval, count);
virResetLastError();
@@ -1361,7 +1359,7 @@ virConnectSetKeepAlive(virConnectPtr conn,
virCheckConnectReturn(conn, -1);
if (conn->driver->connectSetKeepAlive) {
- ret = conn->driver->connectSetKeepAlive(conn, interval, count);
+ int ret = conn->driver->connectSetKeepAlive(conn, interval, count);
if (ret < 0)
goto error;
return ret;
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index f4b140bef4..f5ea973c7a 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -747,7 +747,6 @@ nodeDeviceCreateXML(virConnectPtr conn,
g_autoptr(virNodeDeviceDef) def = NULL;
g_autofree char *wwnn = NULL;
g_autofree char *wwpn = NULL;
- int parent_host = -1;
virNodeDevicePtr device = NULL;
const char *virt_type = NULL;
@@ -765,6 +764,8 @@ nodeDeviceCreateXML(virConnectPtr conn,
return NULL;
if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_SCSI_HOST)) {
+ int parent_host;
+
if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1)
return NULL;
diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
index fffc0f86b7..f474ab32a9 100644
--- a/src/storage/storage_backend_mpath.c
+++ b/src/storage/storage_backend_mpath.c
@@ -151,13 +151,12 @@ static int
virStorageBackendCreateVols(virStoragePoolObjPtr pool,
struct dm_names *names)
{
- int is_mpath = 0;
uint32_t minor = -1;
uint32_t next;
g_autofree char *map_device = NULL;
do {
- is_mpath = virStorageBackendIsMultipath(names->name);
+ int is_mpath = virStorageBackendIsMultipath(names->name);
if (is_mpath < 0)
return -1;
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 23632fc884..7bfcf53570 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2416,7 +2416,6 @@ virStorageBackendVolUploadLocal(virStoragePoolObjPtr pool
G_GNUC_UNUSED,
unsigned int flags)
{
char *target_path = vol->target.path;
- int has_snap = 0;
bool sparse = flags & VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM;
g_autofree char *path = NULL;
@@ -2427,7 +2426,7 @@ virStorageBackendVolUploadLocal(virStoragePoolObjPtr pool
G_GNUC_UNUSED,
* when volUpload is fully finished. */
if (vol->target.format == VIR_STORAGE_FILE_PLOOP) {
/* Fail if the volume contains snapshots or we failed to check it.*/
- has_snap = storageBackendPloopHasSnapshots(vol->target.path);
+ int has_snap = storageBackendPloopHasSnapshots(vol->target.path);
if (has_snap < 0) {
return -1;
} else if (!has_snap) {
@@ -2456,13 +2455,12 @@ virStorageBackendVolDownloadLocal(virStoragePoolObjPtr pool
G_GNUC_UNUSED,
unsigned int flags)
{
char *target_path = vol->target.path;
- int has_snap = 0;
bool sparse = flags & VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM;
g_autofree char *path = NULL;
virCheckFlags(VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM, -1);
if (vol->target.format == VIR_STORAGE_FILE_PLOOP) {
- has_snap = storageBackendPloopHasSnapshots(vol->target.path);
+ int has_snap = storageBackendPloopHasSnapshots(vol->target.path);
if (has_snap < 0) {
return -1;
} else if (!has_snap) {
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index 634e9ac8cd..497b409989 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -1583,7 +1583,6 @@ cmdPoolInfo(vshControl *ctl, const vshCmd *cmd)
virStoragePoolInfo info;
virStoragePoolPtr pool;
int autostart = 0;
- int persistent = 0;
bool ret = true;
bool bytes = false;
char uuid[VIR_UUID_STRING_BUFLEN];
@@ -1601,6 +1600,8 @@ cmdPoolInfo(vshControl *ctl, const vshCmd *cmd)
if (virStoragePoolGetInfo(pool, &info) == 0) {
double val;
const char *unit;
+ int persistent;
+
vshPrint(ctl, "%-15s %s\n", _("State:"),
virshStoragePoolStateToString(info.state));
--
2.26.2
1520
days inactive
1521
days old
30 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Martin Kletzander