Re: [libvirt] [PATCH 1/2] build: use correct type for pid and similar types

No thanks to 64-bit windows, with 64-bit pid_t, we have to avoid constructs like 'int pid'. Our API in libvirt-qemu cannot be changed without breaking ABI; but then again, libvirt-qemu can only be used on systems that support UNIX sockets, which rules out Windows (even if qemu could be compiled there) - so for all points on the call chain that interact with this API decision, we require a different variable name to make it clear that we audited the use for safety. Adding a syntax-check rule only solves half the battle; anywhere that uses printf on a pid_t still needs to be converted, but that will be a separate patch.

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1b92aa4..2e63193 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7925,7 +7926,7 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps, pidfile, monConfig, monJSON))) goto cleanup; - if (virAsprintf(&exepath, "/proc/%u/exe", pid)< 0) { + if (virAsprintf(&exepath, "/proc/%u/exe", (int) pid)< 0) {

virReportOOMError(); goto cleanup; } @@ -7933,7 +7934,7 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps, if (virFileResolveLink(exepath,&emulator)< 0) { virReportSystemError(errno, _("Unable to resolve %s for pid %u"), - exepath, pid); + exepath, (int) pid);

goto cleanup; } VIR_FREE(def->emulator);

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 160cb37..abe73f1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1065,10 +1065,12 @@ qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, int cpu; int ret; + /* In general, we cannot assume pid_t fits in int; but /proc parsing + * is specific to Linux where int works fine. */

if (tid) - ret = virAsprintf(&proc, "/proc/%d/task/%d/stat", pid, tid); + ret = virAsprintf(&proc, "/proc/%d/task/%d/stat", (int) pid, tid); else - ret = virAsprintf(&proc, "/proc/%d/stat", pid); + ret = virAsprintf(&proc, "/proc/%d/stat", (int) pid); if (ret< 0) return -1; @@ -1120,7 +1122,7 @@ qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld", - pid, tid, usertime, systime, cpu, rss); + (int) pid, tid, usertime, systime, cpu, rss); VIR_FORCE_FCLOSE(pidinfo);

diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 3e1a72f..e71dc20 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -94,9 +94,10 @@ static const char * virSecurityDACGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNU } static int -virSecurityDACSetOwnership(const char *path, int uid, int gid) +virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) { - VIR_INFO("Setting DAC user and group on '%s' to '%d:%d'", path, uid, gid); + VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'", + path, (long) uid, (long) gid); if (chown(path, uid, gid)< 0) { struct stat sb; @@ -111,18 +112,22 @@ virSecurityDACSetOwnership(const char *path, int uid, int gid) } if (chown_errno == EOPNOTSUPP || chown_errno == EINVAL) { - VIR_INFO("Setting user and group to '%d:%d' on '%s' not supported by filesystem", - uid, gid, path); + VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not " + "supported by filesystem", + (long) uid, (long) gid, path); } else if (chown_errno == EPERM) { - VIR_INFO("Setting user and group to '%d:%d' on '%s' not permitted", - uid, gid, path); + VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not " + "permitted", + (long) uid, (long) gid, path); } else if (chown_errno == EROFS) { - VIR_INFO("Setting user and group to '%d:%d' on '%s' not possible on readonly filesystem", - uid, gid, path); + VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not " + "possible on readonly filesystem", + (long) uid, (long) gid, path); } else { virReportSystemError(chown_errno, - _("unable to set user and group to '%d:%d' on '%s'"), - uid, gid, path); + _("unable to set user and group to '%ld:%ld' " + "on '%s'"), + (long) uid, (long) gid, path); return -1; } }