In an effort to remove as much gnulib usage as possible let's reimplement virFileReadLink. Since it's used in two places only I opted to open-code it. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 842b70dc26..40a617845a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -13170,12 +13170,13 @@ qemuDomainCreateDeviceRecursive(const char *device, } if (isLink) { + g_autoptr(GError) gerr = NULL; + /* We are dealing with a symlink. Create a dangling symlink and descend * down one level which hopefully creates the symlink's target. */ - if (virFileReadLink(device, &target) < 0) { - virReportSystemError(errno, - _("unable to resolve symlink %s"), - device); + if (!(target = g_file_read_link(device, &gerr))) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to resolve symlink %s: %s"), device, gerr->message); goto cleanup; } @@ -14164,10 +14165,11 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver, data.target = target; } else if (isLink) { - if (virFileReadLink(file, &target) < 0) { - virReportSystemError(errno, - _("unable to resolve symlink %s"), - file); + g_autoptr(GError) gerr = NULL; + + if (!(target = g_file_read_link(file, &gerr))) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to resolve symlink %s: %s"), file, gerr->message); return ret; } -- 2.23.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_tpm.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 62b9f803bd..40136c4410 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -277,15 +277,13 @@ qemuTPMEmulatorGetPid(const char *swtpmStateDir, { int ret; g_autofree char *swtpm = virTPMGetSwtpm(); - char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir, - shortName); + g_autofree char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir, + shortName); if (!pidfile) return -ENOMEM; ret = virPidFileReadPathIfAlive(pidfile, pid, swtpm); - VIR_FREE(pidfile); - return ret; } -- 2.23.0

The caller doesn't care about the actual return value, so return -1 rather than errno. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_vhost_user_gpu.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c index 37e424eac3..51244f9b35 100644 --- a/src/qemu/qemu_vhost_user_gpu.c +++ b/src/qemu/qemu_vhost_user_gpu.c @@ -61,7 +61,7 @@ qemuVhostUserGPUCreatePidFilename(const char *stateDir, * @alias: video device alias * @pid: pointer to pid * - * Return -errno upon error, or zero on successful reading of the pidfile. + * Return -1 upon error, or zero on successful reading of the pidfile. * If the PID was not still alive, zero will be returned, and @pid will be * set to -1; */ @@ -74,11 +74,13 @@ qemuVhostUserGPUGetPid(const char *binPath, { g_autofree char *pidfile = NULL; - pidfile = qemuVhostUserGPUCreatePidFilename(stateDir, shortName, alias); - if (!pidfile) - return -ENOMEM; + if (!(pidfile = qemuVhostUserGPUCreatePidFilename(stateDir, shortName, alias))) + return -1; + + if (virPidFileReadPathIfAlive(pidfile, pid, binPath) < 0) + return -1; - return virPidFileReadPathIfAlive(pidfile, pid, binPath); + return 0; } -- 2.23.0

The callers don't actually use the returned errno for reporting errors. Additionally virFileResolveAllLinks returns -1 rather than -errno on error thus you'd get a spurious EPERM even on other errors. Don't try to return errno in this case. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virpidfile.c | 50 +++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c index b1b8e54993..4a800b6528 100644 --- a/src/util/virpidfile.c +++ b/src/util/virpidfile.c @@ -182,7 +182,7 @@ int virPidFileRead(const char *dir, * If @binpath is NULL the check for the executable path * is skipped. * - * Returns -errno upon error, or zero on successful + * Returns -1 upon error, or zero on successful * reading of the pidfile. If the PID was not still * alive, zero will be returned, but @pid will be * set to -1. @@ -191,8 +191,8 @@ int virPidFileReadPathIfAlive(const char *path, pid_t *pid, const char *binPath) { - int ret; - bool isLink; + int rc; + bool isLink = false; size_t procLinkLen; const char deletedText[] = " (deleted)"; size_t deletedTextLen = strlen(deletedText); @@ -205,16 +205,15 @@ int virPidFileReadPathIfAlive(const char *path, /* only set this at the very end on success */ *pid = -1; - if ((ret = virPidFileReadPath(path, &retPid)) < 0) - return ret; + if (virPidFileReadPath(path, &retPid) < 0) + return -1; #ifndef WIN32 /* Check that it's still alive. Safe to skip this sanity check on * mingw, which lacks kill(). */ if (kill(retPid, 0) < 0) { - ret = 0; - retPid = -1; - goto cleanup; + *pid = -1; + return 0; } #endif @@ -222,23 +221,24 @@ int virPidFileReadPathIfAlive(const char *path, /* we only knew the pid, and that pid is alive, so we can * return it. */ - ret = 0; - goto cleanup; + *pid = retPid; + return 0; } procPath = g_strdup_printf("/proc/%lld/exe", (long long)retPid); - if ((ret = virFileIsLink(procPath)) < 0) - return ret; + if ((rc = virFileIsLink(procPath)) < 0) + return -1; - isLink = ret; + if (rc == 1) + isLink = true; if (isLink && virFileLinkPointsTo(procPath, binPath)) { /* the link in /proc/$pid/exe is a symlink to a file * that has the same inode as the file at binpath. */ - ret = 0; - goto cleanup; + *pid = retPid; + return 0; } /* Even if virFileLinkPointsTo returns a mismatch, it could be @@ -248,24 +248,22 @@ int virPidFileReadPathIfAlive(const char *path, * part, and see if it has the same canonicalized name as binpath. */ if (!(procLink = areadlink(procPath))) - return -errno; + return -1; procLinkLen = strlen(procLink); if (procLinkLen > deletedTextLen) procLink[procLinkLen - deletedTextLen] = 0; - if ((ret = virFileResolveAllLinks(binPath, &resolvedBinPath)) < 0) - return ret; - if ((ret = virFileResolveAllLinks(procLink, &resolvedProcLink)) < 0) - return ret; + if (virFileResolveAllLinks(binPath, &resolvedBinPath) < 0) + return -1; + if (virFileResolveAllLinks(procLink, &resolvedProcLink) < 0) + return -1; - ret = STREQ(resolvedBinPath, resolvedProcLink) ? 0 : -1; + if (STRNEQ(resolvedBinPath, resolvedProcLink)) + return -1; - cleanup: - /* return the originally set pid of -1 unless we proclaim success */ - if (ret == 0) - *pid = retPid; - return ret; + *pid = retPid; + return 0; } -- 2.23.0