[libvirt PATCH 04/14] Remove pointless assignments

Martin Kletzander mkletzan at redhat.com
Thu Sep 24 07:52:11 UTC 2020


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 at 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 at redhat.com>

>     /* It may failed when the machine is not mutable. */
>     rc = gVBoxAPI.UIMachine.GetSettingsFilePath(machine, &settingsFilePath);
>     if (NS_FAILED(rc)) {
>-- 
>2.26.2
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200924/537374a9/attachment-0001.sig>


More information about the libvir-list mailing list