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.

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

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

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

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

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

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