Re: [libvirt] [PATCH 3/5] Introduce functions for checking whether a pidfile is valid

Eh, I forgot to add some more notes... ... > diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c > index 25c3272..dc92868 100644 > --- a/src/util/virpidfile.c > +++ b/src/util/virpidfile.c > @@ -24,6 +24,7 @@ > #include <config.h> > > #include <fcntl.h> > +#include <signal.h> > > #include "virpidfile.h" > #include "virfile.h" > @@ -164,6 +165,63 @@ int virPidFileRead(const char *dir, > } > > > +int virPidFileReadPathIfAlive(const char *path, > + pid_t *pid, > + const char *binpath) > +{ > + int rc; > + char *procpath = NULL; > + > + rc = virPidFileReadPath(path, pid); > + if (rc < 0) > + return rc; > + > + /* Check that it's still alive */ > + if (kill(*pid, 0) < 0) { > + *pid = -1; > + return 0; > + } > + > + if (virAsprintf(&procpath, "/proc/%d/exe", *pid) < 0) { > + *pid = -1; > + return 0; > + } > +#ifdef __linux__ > + if (virFileLinkPointsTo(procpath, binpath) == 0) > + *pid = -1; > +#endif > + VIR_FREE(procpath); Indentation.

And anyway, what about implementing the second half of this function as follows: #ifdef __linux__ if (binpath) { char *procpath = NULL; if (virAsprintf(&procpath, "/proc/%d/exe", *pid) < 0 || !virFileLinkPointsTo(procpath, binpath)) *pid = -1; VIR_FREE(procpath); } #endif

> + > + return 0; > +} I think we should also document that both *IfAlive functions return -errno if the pid file cannot be read or doesn't contain integer value. If it contains a number but such process is not running or doesn't correspond to binpath, the return value is 0 but pid is -1.