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: NMichal Privoznik <mprivozn@redhat.com>

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]$$
 

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")) {

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;
 

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;

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;
         }
     }

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);
 }

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();
     }

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);

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);
 }
 

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,

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) {

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);

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;
     }