[master] 97d187d4d Renovate our pid-file handling.
Poul-Henning Kamp
phk at FreeBSD.org
Thu Mar 7 09:08:08 UTC 2019
commit 97d187d4d659f549e5fcb73b62e4bee22cad10e2
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Thu Mar 7 09:06:13 2019 +0000
Renovate our pid-file handling.
The FreeBSD origin has moved into territory where we will not go
(CAPSICUM) so we can now varnish-ify this without cost.
diff --git a/bin/varnishd/mgt/mgt_main.c b/bin/varnishd/mgt/mgt_main.c
index ceebbf92c..8b1cadcb6 100644
--- a/bin/varnishd/mgt/mgt_main.c
+++ b/bin/varnishd/mgt/mgt_main.c
@@ -839,8 +839,9 @@ main(int argc, char * const *argv)
S_arg = make_secret(dirname);
AN(S_arg);
- assert(!VPF_Write(pfh1));
- assert(pfh2 == NULL || !VPF_Write(pfh2));
+ VPF_Write(pfh1);
+ if (pfh2 != NULL)
+ VPF_Write(pfh2);
MGT_Complain(C_DEBUG, "Version: %s", VCS_version);
MGT_Complain(C_DEBUG, "Platform: %s", VSB_data(vident) + 1);
@@ -922,8 +923,8 @@ main(int argc, char * const *argv)
MGT_Complain(C_INFO, "manager dies");
mgt_cli_close_all();
VEV_Destroy(&mgt_evb);
- (void)VPF_Remove(pfh1);
+ VPF_Remove(pfh1);
if (pfh2 != NULL)
- (void)VPF_Remove(pfh2);
+ VPF_Remove(pfh2);
exit(exit_status);
}
diff --git a/bin/varnishtest/tests/r02646.vtc b/bin/varnishtest/tests/r02646.vtc
index a5c3c432c..8b8b97c08 100644
--- a/bin/varnishtest/tests/r02646.vtc
+++ b/bin/varnishtest/tests/r02646.vtc
@@ -14,8 +14,4 @@ delay 1
shell "rm -f ${tmpdir}/ncsa.pid"
-process p1 -expect-exit 1 -stop -wait
-
-shell -expect "Cannot remove pid file ${tmpdir}/ncsa.pid" {
- cat ${tmpdir}/p1/stderr
-}
+process p1 -stop -wait
diff --git a/bin/varnishtest/vtc_haproxy.c b/bin/varnishtest/vtc_haproxy.c
index 89d0106e3..82ce237bc 100644
--- a/bin/varnishtest/vtc_haproxy.c
+++ b/bin/varnishtest/vtc_haproxy.c
@@ -417,7 +417,7 @@ haproxy_wait_pidfile(struct haproxy *h)
if (vtc_error)
return;
- if (VPF_read(h->pid_fn, &pid) != 0) {
+ if (VPF_Read(h->pid_fn, &pid) != 0) {
bprintf(buf_err,
"Could not read PID file '%s'", h->pid_fn);
usleep(usleep_time);
diff --git a/include/vpf.h b/include/vpf.h
index d5266ac7c..51afaf36f 100644
--- a/include/vpf.h
+++ b/include/vpf.h
@@ -33,9 +33,8 @@
struct vpf_fh;
struct vpf_fh *VPF_Open(const char *path, mode_t mode, pid_t *pidptr);
-int VPF_Write(struct vpf_fh *pfh);
-int VPF_Close(struct vpf_fh *pfh);
-int VPF_Remove(struct vpf_fh *pfh);
-int VPF_read(const char *path, pid_t *);
+int VPF_Read(const char *path, pid_t *);
+void VPF_Write(const struct vpf_fh *pfh);
+void VPF_Remove(struct vpf_fh *pfh);
#endif
diff --git a/lib/libvarnish/vpf.c b/lib/libvarnish/vpf.c
index 0c1bb1f2d..ce406a0dc 100644
--- a/lib/libvarnish/vpf.c
+++ b/lib/libvarnish/vpf.c
@@ -47,13 +47,11 @@
struct vpf_fh {
int pf_fd;
- char pf_path[MAXPATHLEN + 1];
+ char *pf_path;
dev_t pf_dev;
ino_t pf_ino;
};
-static int _VPF_Remove(struct vpf_fh *pfh, int freeit);
-
static int
vpf_verify(const struct vpf_fh *pfh)
{
@@ -72,7 +70,7 @@ vpf_verify(const struct vpf_fh *pfh)
}
int
-VPF_read(const char *path, pid_t *pidptr)
+VPF_Read(const char *path, pid_t *pidptr)
{
char buf[16], *endptr;
int error, fd, i;
@@ -82,8 +80,8 @@ VPF_read(const char *path, pid_t *pidptr)
return (errno);
i = read(fd, buf, sizeof(buf) - 1);
- error = errno; /* Remember errno in case close() wants to change it. */
- (void)close(fd);
+ error = errno;
+ closefd(&fd);
if (i == -1)
return (error);
else if (i == 0)
@@ -104,21 +102,7 @@ VPF_Open(const char *path, mode_t mode, pid_t *pidptr)
{
struct vpf_fh *pfh;
struct stat sb;
- int error, fd, len;
-
- pfh = malloc(sizeof(*pfh));
- if (pfh == NULL)
- return (NULL);
-
- assert(path != NULL);
- len = snprintf(pfh->pf_path, sizeof(pfh->pf_path),
- "%s", path);
-
- if (len >= (int)sizeof(pfh->pf_path)) {
- free(pfh);
- errno = ENAMETOOLONG;
- return (NULL);
- }
+ int fd;
/*
* Open the PID file and obtain exclusive lock.
@@ -126,15 +110,14 @@ VPF_Open(const char *path, mode_t mode, pid_t *pidptr)
* PID file will be truncated again in VPF_Write(), so
* VPF_Write() can be called multiple times.
*/
- fd = VFL_Open(pfh->pf_path,
+ fd = VFL_Open(path,
O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NONBLOCK, mode);
if (fd == -1) {
if (errno == EWOULDBLOCK && pidptr != NULL) {
- errno = VPF_read(pfh->pf_path, pidptr);
+ errno = VPF_Read(path, pidptr);
if (errno == 0)
errno = EEXIST;
}
- free(pfh);
return (NULL);
}
@@ -142,14 +125,12 @@ VPF_Open(const char *path, mode_t mode, pid_t *pidptr)
* Remember file information, so in VPF_Write() we are sure we write
* to the proper descriptor.
*/
- if (fstat(fd, &sb) == -1) {
- error = errno;
- (void)unlink(pfh->pf_path);
- (void)close(fd);
- free(pfh);
- errno = error;
- return (NULL);
- }
+ AZ(fstat(fd, &sb));
+
+ pfh = malloc(sizeof(*pfh));
+ AN(pfh);
+ pfh->pf_path = strdup(path);
+ AN(pfh->pf_path);
pfh->pf_fd = fd;
pfh->pf_dev = sb.st_dev;
@@ -158,99 +139,35 @@ VPF_Open(const char *path, mode_t mode, pid_t *pidptr)
return (pfh);
}
-int
-VPF_Write(struct vpf_fh *pfh)
+void
+VPF_Write(const struct vpf_fh *pfh)
{
char pidstr[16];
- int error, fd;
/*
* Check remembered descriptor, so we don't overwrite some other
* file if pidfile was closed and descriptor reused.
*/
- errno = vpf_verify(pfh);
- if (errno != 0) {
- /*
- * Don't close descriptor, because we are not sure if it's ours.
- */
- return (-1);
- }
- fd = pfh->pf_fd;
+ if (vpf_verify(pfh) != 0)
+ return;
/*
* Truncate PID file, so multiple calls of VPF_Write() are allowed.
*/
- if (ftruncate(fd, 0) == -1) {
- error = errno;
- (void)_VPF_Remove(pfh, 0);
- errno = error;
- return (-1);
- }
+ AZ(ftruncate(pfh->pf_fd, 0));
- error = snprintf(pidstr, sizeof(pidstr), "%jd", (intmax_t)getpid());
- assert(error < sizeof pidstr);
- if (pwrite(fd, pidstr, strlen(pidstr), 0) != (ssize_t)strlen(pidstr)) {
- error = errno;
- (void)_VPF_Remove(pfh, 0);
- errno = error;
- return (-1);
- }
-
- return (0);
+ bprintf(pidstr, "%jd", (intmax_t)getpid());
+ assert(pwrite(pfh->pf_fd, pidstr, strlen(pidstr), 0) ==
+ (ssize_t)strlen(pidstr));
}
-int
-VPF_Close(struct vpf_fh *pfh)
+void
+VPF_Remove(struct vpf_fh *pfh)
{
- int error;
-
- error = vpf_verify(pfh);
- if (error != 0) {
- errno = error;
- return (-1);
+ if (vpf_verify(pfh) == 0) {
+ (void)unlink(pfh->pf_path);
+ closefd(&pfh->pf_fd);
}
-
- if (close(pfh->pf_fd) == -1)
- error = errno;
+ free(pfh->pf_path);
free(pfh);
- if (error != 0) {
- errno = error;
- return (-1);
- }
- return (0);
-}
-
-static int
-_VPF_Remove(struct vpf_fh *pfh, int freeit)
-{
- int error;
-
- error = vpf_verify(pfh);
- if (error != 0) {
- errno = error;
- return (-1);
- }
-
- if (unlink(pfh->pf_path) == -1)
- error = errno;
- if (close(pfh->pf_fd) == -1) {
- if (error == 0)
- error = errno;
- }
- if (freeit)
- free(pfh);
- else
- pfh->pf_fd = -1;
- if (error != 0) {
- errno = error;
- return (-1);
- }
- return (0);
-}
-
-int
-VPF_Remove(struct vpf_fh *pfh)
-{
-
- return (_VPF_Remove(pfh, 1));
}
diff --git a/lib/libvarnishapi/vut.c b/lib/libvarnishapi/vut.c
index 61c200147..91be51925 100644
--- a/lib/libvarnishapi/vut.c
+++ b/lib/libvarnishapi/vut.c
@@ -81,10 +81,7 @@ vut_vpf_remove(void)
AN(pfh);
AN(pfh_vut.P_arg);
- if (VPF_Remove(pfh) != 0)
- VUT_Error(&pfh_vut, 1, "Cannot remove pid file %s: %s",
- pfh_vut.P_arg, strerror(errno));
-
+ VPF_Remove(pfh);
free(pfh_vut.P_arg);
ZERO_OBJ(&pfh_vut, sizeof pfh_vut);
pfh = NULL;
@@ -312,7 +309,7 @@ VUT_Setup(struct VUT *vut)
/* Write PID and setup exit handler */
if (vut->P_arg) {
AN(pfh);
- AZ(VPF_Write(pfh));
+ VPF_Write(pfh);
/* NB: move ownership to a global pseudo-VUT. */
INIT_OBJ(&pfh_vut, VUT_MAGIC);
More information about the varnish-commit
mailing list