The virCommand module is specifically designed so that no caller
has to check for retval of individual virCommand*() APIs except
for virCommandRun() where the actual error is reported. Moreover,
virCommandNew*() use g_new0() to allocate memory and thus it's
not really possible for those APIs to return NULL. Which is why
they are even marked as ATTRIBUTE_NONNULL. But there are few
places where we do check the retval which is a dead code
effectively. Drop those checks.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_slirp.c          |  3 +--
 src/qemu/qemu_tpm.c            | 10 ----------
 src/qemu/qemu_vhost_user_gpu.c |  2 --
 src/qemu/qemu_virtiofs.c       |  3 +--
 src/util/virtpm.c              |  3 +--
 tests/virshtest.c              |  3 +--
 6 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
index c802ef7fa8..3f83db03bf 100644
--- a/src/qemu/qemu_slirp.c
+++ b/src/qemu/qemu_slirp.c
@@ -268,8 +268,7 @@ qemuSlirpStart(virDomainObj *vm,
     if (!(pidfile = qemuSlirpCreatePidFilename(cfg, vm->def, net->info.alias)))
         return -1;
 
-    if (!(cmd = virCommandNew(cfg->slirpHelperName)))
-        return -1;
+    cmd = virCommandNew(cfg->slirpHelperName);
 
     virCommandClearCaps(cmd);
     virCommandSetPidFile(cmd, pidfile);
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 584c787b70..d0aed7fa2e 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -279,8 +279,6 @@ qemuTPMCreateConfigFiles(const char *swtpm_setup)
         return 0;
 
     cmd = virCommandNew(swtpm_setup);
-    if (!cmd)
-        return -1;
 
     virCommandAddArgList(cmd, "--create-config-files", "skip-if-exist", NULL);
     virCommandClearCaps(cmd);
@@ -388,8 +386,6 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
         return -1;
 
     cmd = virCommandNew(swtpm_setup);
-    if (!cmd)
-        return -1;
 
     virUUIDFormat(vmuuid, uuid);
     vmid = g_strdup_printf("%s:%s", vmname, uuid);
@@ -500,8 +496,6 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
         return 0;
 
     cmd = virCommandNew(swtpm_setup);
-    if (!cmd)
-        return -1;
 
     virCommandSetUID(cmd, swtpm_user);
     virCommandSetGID(cmd, swtpm_group);
@@ -592,8 +586,6 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
     unlink(tpm->data.emulator.source->data.nix.path);
 
     cmd = virCommandNew(swtpm);
-    if (!cmd)
-        goto error;
 
     virCommandClearCaps(cmd);
 
@@ -806,8 +798,6 @@ qemuTPMEmulatorStop(const char *swtpmStateDir,
         return;
 
     cmd = virCommandNew(swtpm_ioctl);
-    if (!cmd)
-        return;
 
     virCommandAddArgList(cmd, "--unix", pathname, "-s", NULL);
 
diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c
index 7c5be4098e..bc5a1dc3ec 100644
--- a/src/qemu/qemu_vhost_user_gpu.c
+++ b/src/qemu/qemu_vhost_user_gpu.c
@@ -133,8 +133,6 @@ int qemuExtVhostUserGPUStart(virQEMUDriver *driver,
         goto error;
 
     cmd = virCommandNew(video->driver->vhost_user_binary);
-    if (!cmd)
-        goto error;
 
     virCommandClearCaps(cmd);
     virCommandSetPidFile(cmd, pidfile);
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index ce55286ab5..a04aa08e39 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -132,8 +132,7 @@ qemuVirtioFSBuildCommandLine(virQEMUDriverConfig *cfg,
     g_autoptr(virCommand) cmd = NULL;
     g_auto(virBuffer) opts = VIR_BUFFER_INITIALIZER;
 
-    if (!(cmd = virCommandNew(fs->binary)))
-        return NULL;
+    cmd = virCommandNew(fs->binary);
 
     virCommandAddArgFormat(cmd, "--fd=%d", *fd);
     virCommandPassFD(cmd, *fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 2f2b061fee..91db0f31eb 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -252,8 +252,7 @@ virTPMGetCaps(virTPMBinaryCapsParse capsParse,
 {
     g_autoptr(virCommand) cmd = NULL;
 
-    if (!(cmd = virCommandNew(exec)))
-        return NULL;
+    cmd = virCommandNew(exec);
 
     if (param1)
         virCommandAddArg(cmd, param1);
diff --git a/tests/virshtest.c b/tests/virshtest.c
index a53a6273b9..3d297a1db2 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -116,8 +116,7 @@ testCompareOutputLit(const char *expectData,
     g_autoptr(virCommand) cmd = NULL;
     g_autofree char *errbuf = NULL;
 
-    if (!(cmd = virCommandNewArgs(argv)))
-        return -1;
+    cmd = virCommandNewArgs(argv);
 
     virCommandAddEnvString(cmd, "LANG=C");
     virCommandSetInputBuffer(cmd, empty);
-- 
2.35.1

On 8/23/22 9:51 AM, Michal Privoznik wrote:
> The virCommand module is specifically designed so that no caller
> has to check for retval of individual virCommand*() APIs except
> for virCommandRun() where the actual error is reported. Moreover,
> virCommandNew*() use g_new0() to allocate memory and thus it's
> not really possible for those APIs to return NULL. Which is why
> they are even marked as ATTRIBUTE_NONNULL. But there are few
> places where we do check the retval which is a dead code
> effectively. Drop those checks.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Laine Stump <laine@redhat.com>