tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Allow pidfile(3) to create pidfiles in arbitrary locations



On Fri, Mar 25, 2011 at 01:12:09PM +0100, Jean-Yves Migeon wrote:
> On 23.03.2011 10:32, Julio Merino wrote:
> > Hello,
> > 
> > I would like to change pidfile(3) to support creating pidfiles in arbitrary
> > locations.
> > 
> > Use case: I want to add a flag to a program to specify if a pid file should
> > to be created or not, and I want such flag to carry the path to such pid
> > file.  At the moment, I can't use pidfile(3) for this because it blindly
> > hardcodes /var/run.  (More specifically, I want the pid file to be created
> > in the current directory for the purposes of a test program that should
> > *not* require root privileges.)
> >[snip]
> > Comments?
> 
> None. I just read your patch and did not find any kind of regression
> from previous pidfile(3) behaviour, so looks fine to me.

Thanks for taking a look.

On a second thought, I have updated the change to consider paths with an
explicit '.pid' extension to be considered relative.  Otherwise, I'd expect
confusion if the value to pidfile(3) was provided blindly from a
user-provided parameter (like in foo -p foo.pid) and the pid file was created
in /var/run.  The "only" way to get the old behavior is to just provide a
basename (e.g. foo -p foo).

Updated patch below.


Index: lib/libutil/pidfile.3
===================================================================
RCS file: /cvsroot/src/lib/libutil/pidfile.3,v
retrieving revision 1.12
diff -u -p -r1.12 pidfile.3
--- lib/libutil/pidfile.3       5 May 2010 22:05:31 -0000       1.12
+++ lib/libutil/pidfile.3       29 Mar 2011 08:28:01 -0000
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd May 4, 2010
+.Dd March 29, 2011
 .Dt PIDFILE 3
 .Os
 .Sh NAME
@@ -38,34 +38,52 @@
 .Sh SYNOPSIS
 .In util.h
 .Ft int
-.Fn pidfile "const char *basename"
+.Fn pidfile "const char *path"
 .Sh DESCRIPTION
 .Fn pidfile
-writes a file containing the process ID of the program to the
+creates a file containing the process ID of the caller program.
+The pid file can be used as a quick reference if
+the process needs to be sent a signal.
+When the program exits, the pid file is removed automatically, unless
+the program receives a fatal signal.
+.Pp
+If
+.Ar path
+is
+.Dv NULL
+or a plain basename (a name containing no directory components nor a
+.Sq .pid
+extension), the pid file
+is created in the
 .Pa /var/run
 directory.
 The file name has the form
 .Pa /var/run/basename.pid .
-If the
-.Ar basename
-argument is
+The basename part is either the value of
+.Ar path
+if it was not
 .Dv NULL ,
-.Fn pidfile
-will determine the program name and use that instead.
+or the program name as returned by
+.Xr getprogname 3
+otherwise.
 .Pp
-The pid file can be used as a quick reference if
-the process needs to be sent a signal.
-When the program exits, the pid file will be removed automatically, unless
-the program receives a fatal signal.
+If
+.Ar path
+is an absolute or relative path (i.e. it contains the
+.Sq /
+character or
+it ends with
+.Sq .pid ) ,
+the pid file is created in the provided location.
 .Pp
 Note that only the first invocation of
 .Fn pidfile
 causes a pid file to be written; subsequent invocations have no effect
 unless a new
-.Ar basename
+.Ar path
 is supplied.
 If called with a new
-.Ar basename ,
+.Ar path ,
 .Fn pidfile
 will remove the old pid file and write the new one.
 .Sh RETURN VALUES
@@ -78,11 +96,13 @@ The
 .Fn pidfile
 function call appeared in
 .Nx 1.5 .
+Support for creating pid files in any arbitrary path was added in
+.Nx 6.0 .
 .Sh BUGS
 .Fn pidfile
 uses
 .Xr atexit 3
-to ensure the pidfile is unlinked at program exit.
+to ensure the pid file is unlinked at program exit.
 However, programs that use the
 .Xr _exit 2
 function (for example, in signal handlers)
Index: lib/libutil/pidfile.c
===================================================================
RCS file: /cvsroot/src/lib/libutil/pidfile.c,v
retrieving revision 1.8
diff -u -p -r1.8 pidfile.c
--- lib/libutil/pidfile.c       28 Apr 2008 20:23:03 -0000      1.8
+++ lib/libutil/pidfile.c       29 Mar 2011 08:28:01 -0000
@@ -5,7 +5,7 @@
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
- * by Jason R. Thorpe and Matthias Scheler.
+ * by Jason R. Thorpe, Matthias Scheler and Julio Merino.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -35,86 +35,161 @@ __RCSID("$NetBSD: pidfile.c,v 1.8 2008/0
 #endif
 
 #include <sys/param.h>
+
 #include <paths.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
 #include <util.h>
 
-static int   pidfile_atexit_done;
 static pid_t pidfile_pid;
-static char *pidfile_basename;
 static char *pidfile_path;
 
-static void pidfile_cleanup(void);
+/* Deletes an existent pidfile iff it was created by this process. */
+static void
+pidfile_cleanup(void)
+{
 
-int
-pidfile(const char *basename)
+       if ((pidfile_path != NULL) && (pidfile_pid == getpid()))
+               (void) unlink(pidfile_path);
+}
+
+/* Registers an atexit(3) handler to delete the pidfile we have generated.
+ * We only register the handler when we create a pidfile, so we can assume
+ * that the pidfile exists.
+ *
+ * Returns 0 on success or -1 if the handler could not be registered. */
+static int
+register_atexit_handler(void)
 {
-       FILE *f;
+       static bool done = false;
 
-       /*
-        * Register handler which will remove the pidfile later.
-        */
-       if (!pidfile_atexit_done) {
+       if (!done) {
                if (atexit(pidfile_cleanup) < 0)
                        return -1;
-               pidfile_atexit_done = 1;
+               done = true;
        }
 
-       if (basename == NULL)
-               basename = getprogname();
+       return 0;
+}
 
-       /*
-        * If pidfile has already been created for the supplied basename
-        * we don't need to create a pidfile again.
-        */
+/* Given a new pidfile name in 'path', deletes any previously-created pidfile
+ * if the previous file differs to the new one.
+ *
+ * If a previous file is deleted, returns 1, which means that a new pidfile
+ * must be created.  Otherwise, this returns 0, which means that the existing
+ * file does not need to be touched. */
+static int
+cleanup_old_pidfile(const char* path)
+{
        if (pidfile_path != NULL) {
-               if (strcmp(pidfile_basename, basename) == 0)
+               if (strcmp(pidfile_path, path) != 0) {
+                       pidfile_cleanup();
+
+                       free(pidfile_path);
+                       pidfile_path = NULL;
+
+                       return 1;
+               } else
                        return 0;
-               /*
-                * Remove existing pidfile if it was created by this process.
-                */
-               pidfile_cleanup();
+       } else
+               return 1;
+}
 
-               free(pidfile_path);
-               pidfile_path = NULL;
-               free(pidfile_basename);
-               pidfile_basename = NULL;
-       }
+/* Constructs a name for a pidfile in the default location (/var/run).  If
+ * 'basename' is NULL, uses the name of the current program for the name of
+ * the pidfile.
+ *
+ * Returns a pointer to a dynamically-allocatd string containing the absolute
+ * path to the pidfile; NULL on failure. */
+static char *
+generate_varrun_path(const char *basename)
+{
+       char *path;
 
-       pidfile_pid = getpid();
+       if (basename == NULL)
+               basename = getprogname();
 
-       pidfile_basename = strdup(basename);
-       if (pidfile_basename == NULL)
+       /* _PATH_VARRUN includes trailing / */
+       (void) asprintf(&path, "%s%s.pid", _PATH_VARRUN, basename);
+       return path;
+}
+
+/* Creates a pidfile with the provided name.  The new pidfile is "registered"
+ * in the global variables pidfile_path and pidfile_pid so that any further
+ * call to pidfile(3) can check if we are recreating the same file or a new
+ * one.
+ *
+ * Returns 0 on success or -1 if there is any error. */
+static int
+create_pidfile(const char* path)
+{
+       FILE *f;
+
+       if (register_atexit_handler() == -1)
                return -1;
 
-       /* _PATH_VARRUN includes trailing / */
-       (void) asprintf(&pidfile_path, "%s%s.pid", _PATH_VARRUN, basename);
-       if (pidfile_path == NULL) {
-               free(pidfile_basename);
-               pidfile_basename = NULL;
+       if (cleanup_old_pidfile(path) == 0)
+               return 0;
+
+       pidfile_path = strdup(path);
+       if (pidfile_path == NULL)
                return -1;
-       }
 
-       if ((f = fopen(pidfile_path, "w")) == NULL) {
+       if ((f = fopen(path, "w")) == NULL) {
                free(pidfile_path);
                pidfile_path = NULL;
-               free(pidfile_basename);
-               pidfile_basename = NULL;
                return -1;
        }
 
+       pidfile_pid = getpid();
+
        (void) fprintf(f, "%d\n", pidfile_pid);
        (void) fclose(f);
+
        return 0;
 }
 
-static void
-pidfile_cleanup(void)
+/* Checks if a path to pidfile(3) represents a plain basename or not.
+ * This is to keep backwards compatibility with the original behavior of
+ * pidfile(3), in which calls with a plain basename refer to files in
+ * /var/run.
+ *
+ * Returns true if 'path' is a plain basename; false otherwise. */
+static bool
+is_basename(const char *path)
 {
-       /* Only remove the pidfile if it was created by this process. */
-       if ((pidfile_path != NULL) && (pidfile_pid == getpid()))
-               (void) unlink(pidfile_path);
+       size_t len;
+
+       if (strchr(path, '/') != NULL)
+               return false;
+
+       len = strlen(path);
+       if (len >= 4 && strstr(path, ".pid") == path + (len - 4))
+               return false;
+
+       return true;
+}
+
+int
+pidfile(const char *path)
+{
+
+       if (path == NULL || is_basename(path)) {
+               char *default_path;
+
+               if ((default_path = generate_varrun_path(path)) == NULL)
+                       return -1;
+
+               if (create_pidfile(default_path) == -1) {
+                       free(default_path);
+                       return -1;
+               }
+
+               free(default_path);
+               return 0;
+       } else
+               return create_pidfile(path);
 }
Index: tests/lib/libutil/t_pidfile.c
===================================================================
RCS file: /cvsroot/src/tests/lib/libutil/t_pidfile.c,v
retrieving revision 1.2
diff -u -p -r1.2 t_pidfile.c
--- tests/lib/libutil/t_pidfile.c       23 Mar 2011 09:13:54 -0000      1.2
+++ tests/lib/libutil/t_pidfile.c       29 Mar 2011 08:28:01 -0000
@@ -48,6 +48,7 @@ __COPYRIGHT("@(#) Copyright (c) 2011\
  The NetBSD Foundation, inc. All rights reserved.");
 __RCSID("$NetBSD: t_pidfile.c,v 1.2 2011/03/23 09:13:54 jmmv Exp $");
 
+#include <sys/stat.h>
 #include <sys/wait.h>
 
 #include <assert.h>
@@ -142,7 +143,7 @@ generate_varrun_pidfile(const char *base
 }
 
 static void
-helper_default_basename(const char *path)
+helper_default_path(const char *path)
 {
 
        if (pidfile(NULL) == -1)
@@ -153,17 +154,17 @@ helper_default_basename(const char *path
        exit(EXIT_SUCCESS);
 }
 
-ATF_TC(default_basename);
-ATF_TC_HEAD(default_basename, tc)
+ATF_TC(default_path);
+ATF_TC_HEAD(default_path, tc)
 {
        atf_tc_set_md_var(tc, "require.user", "root");
 }
-ATF_TC_BODY(default_basename, tc)
+ATF_TC_BODY(default_path, tc)
 {
        char *path;
 
        path = generate_varrun_pidfile(NULL);
-       run_child(helper_default_basename, path);
+       run_child(helper_default_path, path);
        ensure_deleted(path);
        free(path);
 }
@@ -196,6 +197,47 @@ ATF_TC_BODY(custom_basename, tc)
 }
 
 static void
+helper_custom_filename(const char *path)
+{
+
+       if (pidfile(path) == -1)
+               errx(EXIT_FAILURE, "Failed to create pidfile '%s'", path);
+       check_pidfile(path);
+       exit(EXIT_SUCCESS);
+}
+
+ATF_TC_WITHOUT_HEAD(custom_filename);
+ATF_TC_BODY(custom_filename, tc)
+{
+
+       run_child(helper_custom_filename, "my-pidfile.pid");
+
+       ensure_deleted("my-pidfile.pid");
+}
+
+static void
+helper_custom_path(const char *path)
+{
+
+       if (pidfile(path) == -1)
+               errx(EXIT_FAILURE, "Failed to create pidfile '%s'", path);
+       check_pidfile(path);
+       exit(EXIT_SUCCESS);
+}
+
+ATF_TC_WITHOUT_HEAD(custom_path);
+ATF_TC_BODY(custom_path, tc)
+{
+
+       ATF_REQUIRE(mkdir("var", 0777) != -1);
+       ATF_REQUIRE(mkdir("var/run", 0777) != -1);
+
+       run_child(helper_custom_path, "./var/run/my-pidfile.pid");
+
+       ensure_deleted("./var/run/my-pidfile.pid");
+}
+
+static void
 helper_change_basenames(const char *unused_cookie)
 {
        char *default_path;
@@ -249,12 +291,93 @@ ATF_TC_BODY(change_basenames, tc)
        free(default_path);
 }
 
+static void
+helper_change_paths(const char *unused_cookie)
+{
+
+       if (pidfile("./var/run/first.pid") == -1)
+               errx(EXIT_FAILURE, "Failed to create pidfile "
+                   "'./var/run/first.pid'");
+       check_pidfile("./var/run/first.pid");
+
+       if (pidfile("./second.pid") == -1)
+               errx(EXIT_FAILURE, "Failed to create pidfile 'second.pid'");
+       ensure_deleted("./var/run/first.pid");
+       check_pidfile("./second.pid");
+
+       exit(EXIT_SUCCESS);
+}
+
+ATF_TC_WITHOUT_HEAD(change_paths);
+ATF_TC_BODY(change_paths, tc)
+{
+
+       ATF_REQUIRE(mkdir("var", 0777) != -1);
+       ATF_REQUIRE(mkdir("var/run", 0777) != -1);
+
+       run_child(helper_change_paths, NULL);
+
+       ensure_deleted("./var/run/my-pidfile.pid");
+       ensure_deleted("second.pid");
+}
+
+static void
+helper_mix(const char *unused_cookie)
+{
+       char *default_path;
+       char *custom_path;
+
+       default_path = generate_varrun_pidfile(NULL);
+       custom_path = generate_varrun_pidfile("custom-basename");
+
+       if (pidfile(NULL) == -1)
+               errx(EXIT_FAILURE, "Failed to create default pidfile");
+       check_pidfile(default_path);
+
+       if (pidfile("./second.pid") == -1)
+               errx(EXIT_FAILURE, "Failed to create pidfile 'second.pid'");
+       ensure_deleted(default_path);
+       check_pidfile("./second.pid");
+
+       if (pidfile("custom-basename") == -1)
+               errx(EXIT_FAILURE, "Failed to create pidfile 'second.pid'");
+       ensure_deleted(default_path);
+       ensure_deleted("./second.pid");
+       ensure_deleted("./custom-basename");
+       check_pidfile(custom_path);
+
+       free(custom_path);
+       free(default_path);
+       exit(EXIT_SUCCESS);
+}
+
+ATF_TC(change_mix);
+ATF_TC_HEAD(change_mix, tc)
+{
+       atf_tc_set_md_var(tc, "require.user", "root");
+}
+ATF_TC_BODY(change_mix, tc)
+{
+       char *default_path;
+
+       run_child(helper_mix, NULL);
+
+       default_path = generate_varrun_pidfile(NULL);
+       ensure_deleted(default_path);
+       ensure_deleted("second.pid");
+       free(default_path);
+}
+
 ATF_TP_ADD_TCS(tp)
 {
 
-       ATF_TP_ADD_TC(tp, default_basename);
+       ATF_TP_ADD_TC(tp, default_path);
        ATF_TP_ADD_TC(tp, custom_basename);
+       ATF_TP_ADD_TC(tp, custom_filename);
+       ATF_TP_ADD_TC(tp, custom_path);
        ATF_TP_ADD_TC(tp, change_basenames);
+       ATF_TP_ADD_TC(tp, change_paths);
+       ATF_TP_ADD_TC(tp, change_mix);
 
        return atf_no_error();
 }


Home | Main Index | Thread Index | Old Index