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
>