src: Treat PID as signed

This initially started as a fix of some debug printing in
virCgroupDetect. However it turned out that other places suffer
from the similar problem. While dealing with pids, esp. in cases
where we cannot use pid_t for ABI stability reasons, we often
chose an unsigned integer type. This makes no sense as pid_t is
signed.
Also, new syntax-check rule is introduced so we won't repeat this
mistake.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/cfg.mk b/cfg.mk
index cc17ec0cf0aa3e3373d61a2a9b2d79e0f53c735b..03099df2e04ef6da59c2190cd6f72fd11dceafa8 100644 (file)
--- a/cfg.mk
+++ b/cfg.mk
@@ -580,6 +580,11 @@ sc_prohibit_int_assign_bool:
        halt='use bool type for boolean values'                         \
          $(_sc_search_regexp)
 
+sc_prohibit_unsigned_pid:
+       @prohibit='\<unsigned\> [^,=;(]+pid'                            \
+       halt='use signed type for pid values'                           \
+         $(_sc_search_regexp)
+
 # Many of the function names below came from this filter:
 # git grep -B2 '\<_('|grep -E '\.c- *[[:alpha:]_][[:alnum:]_]* ?\(.*[,;]$' \
 # |sed 's/.*\.c-  *//'|perl -pe 's/ ?\(.*//'|sort -u \
@@ -1213,6 +1218,9 @@ exclude_file_name_regexp--sc_prohibit_include_public_headers_brackets = \
 exclude_file_name_regexp--sc_prohibit_int_ijk = \
   ^(src/remote_protocol-structs|src/remote/remote_protocol\.x|cfg\.mk|include/libvirt/libvirt.+|src/admin_protocol-structs|src/admin/admin_protocol\.x)$$
 
+exclude_file_name_regexp--sc_prohibit_unsigned_pid = \
+  ^(include/libvirt/.*\.h|src/(qemu/qemu_driver\.c|driver-hypervisor\.h|libvirt(-[a-z]*)?\.c|.*\.x|util/vir(polkit|systemd)\.c)|tests/virpolkittest\.c|tools/virsh-domain\.c)$$
+
 exclude_file_name_regexp--sc_prohibit_getenv = \
   ^tests/.*\.[ch]$$
 

diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index 332c2de96ace74954f2a60cbc9000865b6d84777..280219f72b87af5ec148587a1f12a72060d2d22c 100644 (file)
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -87,7 +87,7 @@ struct _virLockManagerSanlockPrivate {
     char *vm_name;
     unsigned char vm_uuid[VIR_UUID_BUFLEN];
     unsigned int vm_id;
-    unsigned int vm_pid;
+    int vm_pid;
     unsigned int flags;
     bool hasRWDisks;
     int res_count;
@@ -494,7 +494,7 @@ static int virLockManagerSanlockNew(virLockManagerPtr lock,
             if (VIR_STRDUP(priv->vm_name, param->value.str) < 0)
                 goto error;
         } else if (STREQ(param->key, "pid")) {
-            priv->vm_pid = param->value.ui;
+            priv->vm_pid = param->value.iv;
         } else if (STREQ(param->key, "id")) {
             priv->vm_id = param->value.ui;
         } else if (STREQ(param->key, "uri")) {

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 8c581df3dfc099ab850f5d002966bb8ffaa1f8f7..508bc3e6c4488279e04af574ba6084188e91d802 100644 (file)
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1022,7 +1022,7 @@ static void virLXCControllerSignalChildIO(virNetDaemonPtr dmn,
     int status;
 
     ret = waitpid(-1, &status, WNOHANG);
-    VIR_DEBUG("Got sig child %d vs %lld", ret, (unsigned long long)ctrl->initpid);
+    VIR_DEBUG("Got sig child %d vs %lld", ret, (long long) ctrl->initpid);
     if (ret == ctrl->initpid) {
         virNetDaemonQuit(dmn);
         virMutexLock(&lock);
@@ -2328,7 +2328,7 @@ virLXCControllerEventSendInit(virLXCControllerPtr ctrl,
 {
     virLXCMonitorInitEventMsg msg;
 
-    VIR_DEBUG("Init pid %llu", (unsigned long long)initpid);
+    VIR_DEBUG("Init pid %lld", (long long) initpid);
     memset(&msg, 0, sizeof(msg));
     msg.initpid = initpid;
 

diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
index 9027c25640b4b2d24ce15fc415073b9857bfa4c3..3a7404f407c9da34fe50f6bf3a791c84bae3dd7b 100644 (file)
--- a/src/lxc/lxc_domain.c
+++ b/src/lxc/lxc_domain.c
@@ -328,8 +328,8 @@ virLXCDomainObjPrivateXMLFormat(virBufferPtr buf,
 {
     virLXCDomainObjPrivatePtr priv = vm->privateData;
 
-    virBufferAsprintf(buf, "<init pid='%llu'/>\n",
-                      (unsigned long long)priv->initpid);
+    virBufferAsprintf(buf, "<init pid='%lld'/>\n",
+                      (long long) priv->initpid);
 
     return 0;
 }
@@ -340,9 +340,9 @@ virLXCDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
                                virDomainDefParserConfigPtr config ATTRIBUTE_UNUSED)
 {
     virLXCDomainObjPrivatePtr priv = vm->privateData;
-    unsigned long long thepid;
+    long long thepid;
 
-    if (virXPathULongLong("string(./init[1]/@pid)", ctxt, &thepid) < 0) {
+    if (virXPathLongLong("string(./init[1]/@pid)", ctxt, &thepid) < 0) {
         VIR_WARN("Failed to load init pid from state %s",
                  virGetLastErrorMessage());
         priv->initpid = 0;

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index cf30a6638b6f5626123aac872cbf210afc420150..3137018a739ba770376834542ee5b39c91611f29 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3438,7 +3438,7 @@ lxcDomainShutdownFlags(virDomainPtr dom,
             errno != ESRCH) {
             virReportSystemError(errno,
                                  _("Unable to send SIGTERM to init pid %llu"),
-                                 (unsigned long long)priv->initpid);
+                                 (long long) priv->initpid);
             goto endjob;
         }
     }
@@ -3521,7 +3521,7 @@ lxcDomainReboot(virDomainPtr dom,
             errno != ESRCH) {
             virReportSystemError(errno,
                                  _("Unable to send SIGTERM to init pid %llu"),
-                                 (unsigned long long)priv->initpid);
+                                 (long long) priv->initpid);
             goto endjob;
         }
     }

diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c
index 57f40987e4747226a078de173e1d14dbf7309d3d..d828d528a6a6dfd0de2c58f0e5896325ee83cf9a 100644 (file)
--- a/src/lxc/lxc_monitor.c
+++ b/src/lxc/lxc_monitor.c
@@ -105,8 +105,7 @@ virLXCMonitorHandleEventInit(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
     virLXCMonitorPtr mon = opaque;
     virLXCMonitorInitEventMsg *msg = evdata;
 
-    VIR_DEBUG("Event init %llu",
-              (unsigned long long)msg->initpid);
+    VIR_DEBUG("Event init %lld", (long long) msg->initpid);
     if (mon->cb.initNotify)
         mon->cb.initNotify(mon, (pid_t)msg->initpid, mon->vm);
 }

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index bce6a2f70ea892cbe859ee25d08ce9ec647dbcb7..07da3d3d06f8f6eb9e2f547b6f84c3c5eb42bf93 100644 (file)
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -740,8 +740,8 @@ virLXCProcessGetNsInode(pid_t pid,
     struct stat sb;
     int ret = -1;
 
-    if (virAsprintf(&path, "/proc/%llu/ns/%s",
-                    (unsigned long long)pid, nsname) < 0)
+    if (virAsprintf(&path, "/proc/%lld/ns/%s",
+                    (long long) pid, nsname) < 0)
         goto cleanup;
 
     if (stat(path, &sb) < 0) {
@@ -776,8 +776,8 @@ static void virLXCProcessMonitorInitNotify(virLXCMonitorPtr mon ATTRIBUTE_UNUSED
     priv->initpid = initpid;
 
     if (virLXCProcessGetNsInode(initpid, "pid", &inode) < 0) {
-        VIR_WARN("Cannot obtain pid NS inode for %llu: %s",
-                 (unsigned long long)initpid,
+        VIR_WARN("Cannot obtain pid NS inode for %lld: %s",
+                 (long long) initpid,
                  virGetLastErrorMessage());
         virResetLastError();
     }

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bdf88003059fb2d9fcfd972570be266f1cf7b4fc..85795345d1ec983569af69e82c1b43616ea7912f 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5444,8 +5444,8 @@ qemuProcessLaunch(virConnectPtr conn,
                            _("Domain %s didn't show up"), vm->def->name);
             rv = -1;
         }
-        VIR_DEBUG("QEMU vm=%p name=%s running with pid=%llu",
-                  vm, vm->def->name, (unsigned long long)vm->pid);
+        VIR_DEBUG("QEMU vm=%p name=%s running with pid=%lld",
+                  vm, vm->def->name, (long long) vm->pid);
     } else {
         VIR_DEBUG("QEMU vm=%p name=%s failed to spawn",
                   vm, vm->def->name);
@@ -5836,9 +5836,9 @@ qemuProcessKill(virDomainObjPtr vm, unsigned int flags)
 {
     int ret;
 
-    VIR_DEBUG("vm=%p name=%s pid=%llu flags=%x",
+    VIR_DEBUG("vm=%p name=%s pid=%lld flags=%x",
               vm, vm->def->name,
-              (unsigned long long)vm->pid, flags);
+              (long long) vm->pid, flags);
 
     if (!(flags & VIR_QEMU_PROCESS_KILL_NOCHECK)) {
         if (!virDomainObjIsActive(vm)) {
@@ -5916,10 +5916,10 @@ void qemuProcessStop(virQEMUDriverPtr driver,
     char *timestamp;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 
-    VIR_DEBUG("Shutting down vm=%p name=%s id=%d pid=%llu, "
+    VIR_DEBUG("Shutting down vm=%p name=%s id=%d pid=%lld, "
               "reason=%s, asyncJob=%s, flags=%x",
               vm, vm->def->name, vm->def->id,
-              (unsigned long long)vm->pid,
+              (long long) vm->pid,
               virDomainShutoffReasonTypeToString(reason),
               qemuDomainAsyncJobTypeToString(asyncJob),
               flags);

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 8b52816adf384e81b26e3ee6885d952e1608f6fc..24917e71506d8b14664cd944c82f271c3d300a22 100644 (file)
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -548,13 +548,13 @@ virCgroupDetectPlacement(virCgroupPtr group,
     char *procfile;
 
     VIR_DEBUG("Detecting placement for pid %lld path %s",
-              (unsigned long long)pid, path);
+              (long long) pid, path);
     if (pid == -1) {
         if (VIR_STRDUP(procfile, "/proc/self/cgroup") < 0)
             goto cleanup;
     } else {
-        if (virAsprintf(&procfile, "/proc/%llu/cgroup",
-                        (unsigned long long)pid) < 0)
+        if (virAsprintf(&procfile, "/proc/%lld/cgroup",
+                        (long long) pid) < 0)
             goto cleanup;
     }
 
@@ -732,11 +732,12 @@ virCgroupDetect(virCgroupPtr group,
             return -1;
         }
 
-        VIR_DEBUG("Detected mount/mapping %zu:%s at %s in %s for pid %llu", i,
+        VIR_DEBUG("Detected mount/mapping %zu:%s at %s in %s for pid %lld",
+                  i,
                   virCgroupControllerTypeToString(i),
                   group->controllers[i].mountPoint,
                   group->controllers[i].placement,
-                  (unsigned long long)pid);
+                  (long long) pid);
     }
 
     return 0;
@@ -1235,8 +1236,7 @@ virCgroupAddTaskController(virCgroupPtr group, pid_t pid, int controller)
         return -1;
     }
 
-    return virCgroupSetValueU64(group, controller, "tasks",
-                                (unsigned long long)pid);
+    return virCgroupSetValueI64(group, controller, "tasks", pid);
 }
 
 
@@ -3506,8 +3506,8 @@ virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr pids)
             goto cleanup;
         } else {
             while (!feof(fp)) {
-                unsigned long pid_value;
-                if (fscanf(fp, "%lu", &pid_value) != 1) {
+                long pid_value;
+                if (fscanf(fp, "%ld", &pid_value) != 1) {
                     if (feof(fp))
                         break;
                     virReportSystemError(errno,
@@ -3518,12 +3518,12 @@ virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr pids)
                 if (virHashLookup(pids, (void*)pid_value))
                     continue;
 
-                VIR_DEBUG("pid=%lu", pid_value);
+                VIR_DEBUG("pid=%ld", pid_value);
                 /* Cgroups is a Linux concept, so this cast is safe.  */
                 if (kill((pid_t)pid_value, signum) < 0) {
                     if (errno != ESRCH) {
                         virReportSystemError(errno,
-                                             _("Failed to kill process %lu"),
+                                             _("Failed to kill process %ld"),
                                              pid_value);
                         goto cleanup;
                     }
@@ -3553,7 +3553,7 @@ virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr pids)
 static uint32_t
 virCgroupPidCode(const void *name, uint32_t seed)
 {
-    unsigned long pid_value = (unsigned long)(intptr_t)name;
+    long pid_value = (long)(intptr_t)name;
     return virHashCodeGen(&pid_value, sizeof(pid_value), seed);
 }
 

diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 9b8ba4ae245728de2665f9908e32243974793915..52a0a30a4520807dcfec60c9fbd94b5756f6b713 100644 (file)
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -505,7 +505,7 @@ int virIdentitySetUNIXProcessID(virIdentityPtr ident,
 {
     char *val;
     int ret;
-    if (virAsprintf(&val, "%llu", (unsigned long long)pid) < 0)
+    if (virAsprintf(&val, "%lld", (long long) pid) < 0)
         return -1;
     ret = virIdentitySetAttr(ident,
                              VIR_IDENTITY_ATTR_UNIX_PROCESS_ID,

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 8b9f9c5d609a8aa75a71c8d77f9ed31c727edf34..718c4a2664e1e8f865eaf1a1572fcb358ce101c4 100644 (file)
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -608,8 +608,7 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids)
     *npids = 0;
     *pids = NULL;
 
-    if (virAsprintf(&taskPath, "/proc/%llu/task",
-                    (unsigned long long)pid) < 0)
+    if (virAsprintf(&taskPath, "/proc/%llu/task", (long long) pid) < 0)
         goto cleanup;
 
     if (virDirOpen(&dir, taskPath) < 0)
@@ -657,7 +656,7 @@ int virProcessGetNamespaces(pid_t pid,
         int fd;
 
         if (virAsprintf(&nsfile, "/proc/%llu/ns/%s",
-                        (unsigned long long)pid,
+                        (long long) pid,
                         ns[i]) < 0)
             goto cleanup;
 
@@ -968,8 +967,7 @@ int virProcessGetStartTime(pid_t pid,
     int len;
     char **tokens = NULL;
 
-    if (virAsprintf(&filename, "/proc/%llu/stat",
-                    (unsigned long long)pid) < 0)
+    if (virAsprintf(&filename, "/proc/%llu/stat", (long long) pid) < 0)
         return -1;
 
     if ((len = virFileReadAll(filename, 1024, &buf)) < 0)
@@ -1051,8 +1049,8 @@ int virProcessGetStartTime(pid_t pid,
 {
     static int warned;
     if (virAtomicIntInc(&warned) == 1) {
-        VIR_WARN("Process start time of pid %llu not available on this platform",
-                 (unsigned long long)pid);
+        VIR_WARN("Process start time of pid %lld not available on this platform",
+                 (long long) pid);
     }
     *timestamp = 0;
     return 0;
@@ -1069,7 +1067,7 @@ static int virProcessNamespaceHelper(int errfd,
     int fd = -1;
     int ret = -1;
 
-    if (virAsprintf(&path, "/proc/%llu/ns/mnt", (unsigned long long)pid) < 0)
+    if (virAsprintf(&path, "/proc/%lld/ns/mnt", (long long) pid) < 0)
         goto cleanup;
 
     if ((fd = open(path, O_RDONLY)) < 0) {

diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 604dcdd0ad24747a8b443686664006c3f7afd69f..0219db6ec4430dfd8e4384324ff05b97eb308fbc 100644 (file)
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -210,8 +210,8 @@ virSystemdGetMachineNameByPID(pid_t pid)
     if (virDBusMessageRead(reply, "o", &object) < 0)
         goto cleanup;
 
-    VIR_DEBUG("Domain with pid %llu has object path '%s'",
-              (unsigned long long)pid, object);
+    VIR_DEBUG("Domain with pid %lld has object path '%s'",
+              (long long) pid, object);
 
     if (virDBusCallMethod(conn, &reply, NULL,
                           "org.freedesktop.machine1",
@@ -226,8 +226,8 @@ virSystemdGetMachineNameByPID(pid_t pid)
     if (virDBusMessageRead(reply, "v", "s", &name) < 0)
         goto cleanup;
 
-    VIR_DEBUG("Domain with pid %llu has machine name '%s'",
-              (unsigned long long)pid, name);
+    VIR_DEBUG("Domain with pid %lld has machine name '%s'",
+              (long long) pid, name);
 
  cleanup:
     VIR_FREE(object);

diff --git a/src/util/virutil.c b/src/util/virutil.c
index b57a1956c494f59dc594e2ca5411c51b56a25402..2d46b7ee3bce4dd6733a44356857cd74ecf40192 100644 (file)
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -2582,8 +2582,8 @@ virGetListenFDs(void)
     }
 
     if ((pid_t)procid != getpid()) {
-        VIR_DEBUG("LISTEN_PID %s is not for us %llu",
-                  pidstr, (unsigned long long)getpid());
+        VIR_DEBUG("LISTEN_PID %s is not for us %lld",
+                  pidstr, (long long) getpid());
         return 0;
     }