On 12/13/21 1:58 PM, Ján Tomko wrote:
Use two variables with automatic cleanup instead of reusing one.
Remove the pointless cleanup label.
Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
---
src/util/virdnsmasq.c | 37 ++++++++++++++++---------------------
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 2dd9a20377..b62e353ceb 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -666,9 +666,9 @@ dnsmasqCapsSetFromBuffer(dnsmasqCaps *caps, const char *buf)
static int
dnsmasqCapsRefreshInternal(dnsmasqCaps *caps, bool force)
{
- int ret = -1;
struct stat sb;
- virCommand *cmd = NULL;
+ g_autoptr(virCommand) vercmd = NULL;
+ g_autoptr(virCommand) helpcmd = NULL;
g_autofree char *help = NULL;
g_autofree char *version = NULL;
g_autofree char *complete = NULL;
@@ -692,31 +692,26 @@ dnsmasqCapsRefreshInternal(dnsmasqCaps *caps, bool force)
if (!virFileIsExecutable(caps->binaryPath)) {
virReportSystemError(errno, _("dnsmasq binary %s is not executable"),
caps->binaryPath);
- goto cleanup;
+ return -1;
}
- cmd = virCommandNewArgList(caps->binaryPath, "--version", NULL);
- virCommandSetOutputBuffer(cmd, &version);
- virCommandAddEnvPassCommon(cmd);
- virCommandClearCaps(cmd);
- if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
- virCommandFree(cmd);
+ vercmd = virCommandNewArgList(caps->binaryPath, "--version", NULL);
+ virCommandSetOutputBuffer(vercmd, &version);
+ virCommandAddEnvPassCommon(vercmd);
+ virCommandClearCaps(vercmd);
+ if (virCommandRun(vercmd, NULL) < 0)
+ return -1;
Hmmm. Every time I run across this code, I wonder if we should keep it
or just remove it completely - the "newest" feature we're checking for
was added to dnsmasq in version 2.67, which was released in late 2013.
So all these extra executions of dnsmasq to get the version# and parse
the help output are just producing the same results for everyone.
On the other hand, it's possible some new feature could be added to
dnsmasq in the future that we would want to check for, and that would be
easier to add if the basic structure of the code was still here. I'm not
sure how likely that is at this point though - dnsmasq (and libvirt's
use of dnsmasq) is fairly mature at this point, so keeping the code is
just creating more maintenance burden for nothing...
