[PATCH] chpst in/out/err enhancement

[PATCH] chpst in/out/err enhancement

[Mike Pomraning]

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

The attached patch:

1. Preserves complaints to standard error even under '-2'
2. Adds a '-N' ("null device") flag which modifies '-[012]' to
redirect /dev/null with appropriate access modes, rather than merely
closing fds
3. Documents #2

Rationale:  #1 above helps me debug otherwise quiet ``chpst -2
/oops/enoent'' typographical errors.  #2 guards against some programs'
naive assumptions about the availability and nature of fds 0, 1 and 2.

Implementation notes:  The '-N' flag redirects before chroot, and
respects the improved semantics of '-2' (that is, chpst can still
complain of exec failures).  Unmodified '-[012]' are now implemented
with close-on-exec rather than close().  Other than the possibility of
new complaints to stderr under '-2', the patch is behaviorally
backward compatible.

Let me know if you think this is wrong-headed.

-Mike

Re: [PATCH] chpst in/out/err enhancement

[Charlie Brady]

On Thu, 27 Sep 2012, Mike Pomraning wrote:

> The attached patch:

I'm guessing it was stripped by the maillist list software. Perhaps you 
could post it in-line.

Re: [PATCH] chpst in/out/err enhancement

[Mike Pomraning]

Thanks.  Try #2, inline.  Also available here
<http://pilcrow.madison.wi.us/sw/runit-2.1.1-devnull-20120912.patch>

-Mike

diff -up runit-2.1.1/doc/chpst.8.html.orig runit-2.1.1/doc/chpst.8.html
--- runit-2.1.1/doc/chpst.8.html.orig	2009-10-04 15:44:02.000000000 -0500
+++ runit-2.1.1/doc/chpst.8.html	2012-09-12 09:23:11.000000000 -0500
@@ -106,16 +106,24 @@ This includes warnings about limits unsu
 <dd>pgrphack.
 Run <i>prog</i> in a new process group. </dd>

+<dt><b>-N</b> </dt>
+<dd>Interpret <i>-0</i>, <i>-1</i> and <i>-2</i> as reopening standard
+I/O streams to <i>/dev/null</i>, rather than as closing the streams. </dd>
+
 <dt><b>-0</b> </dt>
-<dd>Close standard input before starting
+<dd>Close standard input (default), or reopen it to
+<i>/dev/null</i> (<i>-N</i>), before starting
 <i>prog</i>. </dd>

 <dt><b>-1</b> </dt>
-<dd>Close standard output before starting <i>prog</i>. </dd>
+<dd>Close standard output (default), or reopen it to
+<i>/dev/null</i> (<i>-N</i>), before starting
+<i>prog</i>. </dd>

 <dt><b>-2</b> </dt>
-<dd>Close standard error
-before starting <i>prog</i>. </dd>
+<dd>Close standard error (default), or reopen it to
+<i>/dev/null</i> (<i>-N</i>), before starting
+<i>prog</i>. </dd>
 </dl>

 <h2><a name='sect4'>Exit Codes</a></h2>
diff -up runit-2.1.1/man/chpst.8.orig runit-2.1.1/man/chpst.8
--- runit-2.1.1/man/chpst.8.orig	2009-10-04 15:44:02.000000000 -0500
+++ runit-2.1.1/man/chpst.8	2012-09-12 09:26:53.000000000 -0500
@@ -3,7 +3,7 @@
 chpst \- runs a program with a changed process state
 .SH SYNOPSIS
 .B chpst
-[\-vP012]
+[\-vPN012]
 [\-u
 .IR user ]
 [\-U
@@ -219,16 +219,35 @@ Run
 .I prog
 in a new process group.
 .TP
+.B \-N
+Interpret
+.IR \-0 ,
+.IR \-1
+and
+.IR \-2
+as reopening standard I/O streams to
+.IR /dev/null ,
+rather than as closing the streams.
+.TP
 .B \-0
-Close standard input before starting
+Close standard input (default), or reopen it to
+.IR /dev/null
+(\fI\-N\fP),
+before starting
 .IR prog .
 .TP
 .B \-1
-Close standard output before starting
+Close standard output (default), or reopen it to
+.IR /dev/null
+(\fI\-N\fP),
+before starting
 .IR prog .
 .TP
 .B \-2
-Close standard error before starting
+Close standard error (default), or reopen it to
+.IR /dev/null
+(\fI\-N\fP),
+before starting
 .IR prog .
 .SH EXIT CODES
 .B chpst
diff -up runit-2.1.1/package/CHANGES.orig runit-2.1.1/package/CHANGES
--- runit-2.1.1/package/CHANGES.orig	2009-10-04 15:44:02.000000000 -0500
+++ runit-2.1.1/package/CHANGES	2012-09-12 10:00:18.000000000 -0500
@@ -1,3 +1,9 @@
+2.1.1-devnull.patch
+Wed, 12 Sep 2012 14:57:01 +0000
+  * chpst.c, man/chpst.8: new option -N: change -0, -1, -2 to reopen to
+    /dev/null, rather than close.  Also, -2 no longer suppresses error
+    messages pre-exec(). (Mike Pomraning)
+
 2.1.1
 Sun, 04 Oct 2009 20:28:38 +0000
   * doc/upgrade.html: fix typo.
diff -up runit-2.1.1/src/chpst.c.orig runit-2.1.1/src/chpst.c
--- runit-2.1.1/src/chpst.c.orig	2009-10-04 15:44:02.000000000 -0500
+++ runit-2.1.1/src/chpst.c	2012-09-12 08:42:08.000000000 -0500
@@ -19,8 +19,11 @@
 #include "open.h"
 #include "openreadclose.h"
 #include "direntry.h"
+#include "fd.h"
+#include "coe.h"
+#include "buffer.h"

-#define USAGE_MAIN " [-vP012] [-u user[:group]] [-U user[:group]] [-b
argv0] [-e dir] [-/ root] [-n nice] [-l|-L lock] [-m n] [-d n] [-o n]
[-p n] [-f n] [-c n] prog"
+#define USAGE_MAIN " [-vPN012] [-u user[:group]] [-U user[:group]]
[-b argv0] [-e dir] [-/ root] [-n nice] [-l|-L lock] [-m n] [-d n] [-o
n] [-p n] [-f n] [-c n] prog"
 #define FATAL "chpst: fatal: "
 #define WARNING "chpst: warning: "

@@ -44,6 +47,7 @@ const char *argv0 =0;
 const char *env_dir =0;
 unsigned int verbose =0;
 unsigned int pgrp =0;
+unsigned int devnull =0;
 unsigned int nostdin =0;
 unsigned int nostdout =0;
 unsigned int nostderr =0;
@@ -257,6 +261,46 @@ void slimit() {
   }
 }

+void coe012(int in, int out, int err) {
+  if (in) if (coe(0) == -1)
+    fatal("unable to set close-on-exec on standard input");
+  if (out) if (coe(1) == -1)
+    fatal("unable to set close-on-exec on standard output");
+  if (err) if (coe(2) == -1)
+    fatal("unable to set close-on-exec on standard error");
+}
+
+void dn012(int in, int out, int err) {
+  int dn;
+
+  if (in) {
+    if ((dn =open_read("/dev/null")) == -1)
+      fatal("unable to open /dev/null for reading");
+    if (fd_move(0, dn) == -1)
+      fatal("unable to duplicate /dev/null to standard input");
+  }
+
+  if (out || err) {
+    if ((dn =open_write("/dev/null")) == -1)
+      fatal("unable to open /dev/null for writing");
+
+    if (out) if (fd_copy(1, dn) == -1)
+      fatal("unable to duplicate /dev/null to standard output");
+
+    if (err) {
+      int newerr =fd_dup(2);
+      if (newerr == -1) fatal("unable to duplicate standard error");
+      if (coe(newerr) == -1)
+        fatal("unable to set close-on-exec on duplicated standard error");
+      buffer_2->fd = newerr;
+      if (fd_copy(2, dn) == -1)
+        fatal("unable to duplicate /dev/null to standard error");
+    }
+
+    close(dn);
+  }
+}
+
 /* argv[0] */
 void setuidgid(int, const char *const *);
 void envuidgid(int, const char *const *);
@@ -286,7 +330,7 @@ int main(int argc, const char **argv) {
   if (str_equal(progname, "setlock")) setlock(argc, argv);
   if (str_equal(progname, "softlimit")) softlimit(argc, argv);

-  while ((opt =getopt(argc, argv, "u:U:b:e:m:d:o:p:f:c:r:t:/:n:l:L:vP012V"))
+  while ((opt =getopt(argc, argv, "u:U:b:e:m:d:o:p:f:c:r:t:/:n:l:L:vPN012V"))
          != opteof)
     switch(opt) {
     case 'u': set_user =(char*)optarg; break;
@@ -321,6 +365,7 @@ int main(int argc, const char **argv) {
     case 'L': if (lock) usage(); lock =optarg; lockdelay =0; break;
     case 'v': verbose =1; break;
     case 'P': pgrp =1; break;
+    case 'N': devnull =1; break;
     case '0': nostdin =1; break;
     case '1': nostdout =1; break;
     case '2': nostderr =1; break;
@@ -332,6 +377,7 @@ int main(int argc, const char **argv) {

   if (pgrp) setsid();
   if (env_dir) edir(env_dir);
+  (devnull?dn012:coe012)(nostdin, nostdout, nostderr);
   if (root) {
     if (chdir(root) == -1) fatal2("unable to change directory", root);
     if (chroot(".") == -1) fatal("unable to change root directory");
@@ -343,9 +389,6 @@ int main(int argc, const char **argv) {
   if (env_user) euidgid(env_user, 1);
   if (set_user) suidgid(set_user, 1);
   if (lock) slock(lock, lockdelay, 0);
-  if (nostdin) if (close(0) == -1) fatal("unable to close stdin");
-  if (nostdout) if (close(1) == -1) fatal("unable to close stdout");
-  if (nostderr) if (close(2) == -1) fatal("unable to close stderr");
   slimit();

   progname =*argv;
diff -up runit-2.1.1/src/fd_dup.c.orig runit-2.1.1/src/fd_dup.c
--- runit-2.1.1/src/fd_dup.c.orig	2012-09-11 23:58:31.000000000 -0500
+++ runit-2.1.1/src/fd_dup.c	2012-09-12 00:10:35.000000000 -0500
@@ -0,0 +1,9 @@
+/* Public domain addition to DJB API. */
+
+#include <fcntl.h>
+#include "fd.h"
+
+int fd_dup(int fd)
+{
+  return fcntl(fd, F_DUPFD, 0);
+}
diff -up runit-2.1.1/src/fd.h.orig runit-2.1.1/src/fd.h
--- runit-2.1.1/src/fd.h.orig	2009-10-04 15:44:02.000000000 -0500
+++ runit-2.1.1/src/fd.h	2012-09-12 00:06:41.000000000 -0500
@@ -4,6 +4,7 @@
 #define FD_H

 extern int fd_copy(int,int);
+extern int fd_dup(int);      /* Addition to DJB API */
 extern int fd_move(int,int);

 #endif
diff -up runit-2.1.1/src/Makefile.orig runit-2.1.1/src/Makefile
--- runit-2.1.1/src/Makefile.orig	2009-10-04 15:44:02.000000000 -0500
+++ runit-2.1.1/src/Makefile	2012-09-12 10:38:27.000000000 -0500
@@ -202,6 +202,9 @@ error.o: compile error.c error.h
 error_str.o: compile error.h error_str.c
 	./compile error_str.c

+fd_dup.o: compile fd.h fd_dup.c
+	./compile fd_dup.c
+
 fd_copy.o: compile fd.h fd_copy.c
 	./compile fd_copy.c

@@ -441,7 +444,7 @@ uint64.h: choose compile load tryulong64

 unix.a: alloc.o alloc_re.o buffer.o buffer_0.o buffer_1.o buffer_2.o \
 buffer_get.o buffer_put.o buffer_read.o buffer_write.o coe.o env.o \
-error.o error_str.o fd_copy.o fd_move.o fifo.o lock_ex.o lock_exnb.o \
+error.o error_str.o fd_copy.o fd_dup.o fd_move.o fifo.o lock_ex.o lock_exnb.o \
 makelib ndelay_off.o ndelay_on.o open_append.o open_read.o \
 open_trunc.o open_write.o openreadclose.o pathexec_env.o \
 pathexec_run.o prot.o readclose.o seek_set.o sgetopt.o sig.o \
@@ -451,14 +454,13 @@ stralloc_pend.o strerr_die.o strerr_sys.
 wait_pid.o
 	./makelib unix.a alloc.o alloc_re.o buffer.o buffer_0.o buffer_1.o \
 	buffer_2.o buffer_get.o buffer_put.o buffer_read.o buffer_write.o \
-	coe.o env.o error.o error_str.o fd_copy.o fd_move.o fifo.o lock_ex.o \
-	lock_exnb.o ndelay_off.o ndelay_on.o open_append.o open_read.o \
-	open_trunc.o open_write.o openreadclose.o pathexec_env.o \
-	pathexec_run.o prot.o readclose.o seek_set.o sgetopt.o sig.o \
-	sig_block.o sig_catch.o sig_pause.o stralloc_cat.o stralloc_catb.o \
-	stralloc_cats.o stralloc_eady.o stralloc_opyb.o stralloc_opys.o \
-	stralloc_pend.o strerr_die.o strerr_sys.o subgetopt.o wait_nohang.o \
-	wait_pid.o
+	coe.o env.o error.o error_str.o fd_copy.o fd_dup.o fd_move.o fifo.o \
+	lock_ex.o lock_exnb.o ndelay_off.o ndelay_on.o open_append.o open_read.o \
+	open_trunc.o open_write.o openreadclose.o pathexec_env.o pathexec_run.o \
+	prot.o readclose.o seek_set.o sgetopt.o sig.o sig_block.o sig_catch.o \
+	sig_pause.o stralloc_cat.o stralloc_catb.o stralloc_cats.o stralloc_eady.o \
+	stralloc_opyb.o stralloc_opys.o stralloc_pend.o strerr_die.o strerr_sys.o \
+	subgetopt.o wait_nohang.o wait_pid.o

 wait_nohang.o: compile haswaitp.h wait_nohang.c
 	./compile wait_nohang.c


On Thu, Sep 27, 2012 at 4:22 PM, Charlie Brady
<charlieb-supervision@budge.apana.org.au> wrote:
>
> On Thu, 27 Sep 2012, Mike Pomraning wrote:
>
>> The attached patch:
>
> I'm guessing it was stripped by the maillist list software. Perhaps you
> could post it in-line.