[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [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 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 redhat com>

    /* It may failed when the machine is not mutable. */
    rc = gVBoxAPI.UIMachine.GetSettingsFilePath(machine, &settingsFilePath);
    if (NS_FAILED(rc)) {
--
2.26.2

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]