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>
This commit is contained in:
Michal Privoznik 2016-10-06 16:54:41 +02:00
parent 83adcb5bdf
commit b7d2d4af2b
13 changed files with 54 additions and 49 deletions

8
cfg.mk
View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -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,

View File

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

View File

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

View File

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