Attached is a slightly-reworked version of the earlier patch that adds
stderr handling to watch.  This version makes the stderr handling
default rather than optional.  I prefer this version of the patch,
because of the principle of least surprise in UI design, reference
http://en.wikipedia.org/wiki/Principle_of_least_astonishment .  When
one runs a command like "watch ls -ld some_file", and then some_file
is deleted, one is not expecting a bunch of scrolling errors to
obscure the watch command.

I see at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=225549 that
Justin Pryzby thinks dup2(STDOUT_FILENO, STDERR_FILENO) is a bad idea
because it loses the ability to discard stderr.  Note that it's still
possible for users to override the default handling inside the
command, i.e.:

  watch "ls -ld no_such_file 2>/dev/null"

So anyone who wants to discard stderr still can.  And if the desire to
add "discard stderr" as a native watch capability comes up, it's still
possible to add a "discard stderr" option even with the default of
dup2(1, 2).  I'd be happy to do this if a "discard stderr" option is
needed for this functionality to be added to watch.

- Morty
diff -Nur procps-3.2.7.orig/watch.1 procps-3.2.7/watch.1
--- procps-3.2.7.orig/watch.1   2003-02-09 02:05:25.000000000 -0500
+++ procps-3.2.7/watch.1        2008-08-08 16:26:53.000000000 -0400
@@ -3,12 +3,13 @@
 watch \- execute a program periodically, showing output fullscreen
 .SH SYNOPSIS
 .B watch
-.I [\-dhvt] [\-n <seconds>] [\-\-differences[=cumulative]] [\-\-help] 
[\-\-interval=<seconds>] [\-\-no\-title] [\-\-version] <command>
+.I [\-bdhvt] [\-n <seconds>] [\-\-beep] [\-\-differences[=cumulative]] 
[\-\-help] [\-\-interval=<seconds>] [\-\-no\-title] [\-\-version] <command>
 .SH DESCRIPTION
 .BR watch
 runs
 .I command
-repeatedly, displaying its output (the first screenfull).  This allows you to
+repeatedly, displaying its output and errors (the first screenfull).  This 
+allows you to
 watch the program output change over time.  By default, the program is run
 every 2 seconds; use 
 .I -n
@@ -28,7 +29,11 @@
 or
 .I --no-title
 option turns off the header showing the interval, command, and current
-time at the top of the display, as well as the following blank line.
+time at the top of the display, as well as the following blank line.  The
+.I -b
+or 
+.I --beep
+option causes the command to beep if it has a non-zero exit.
 .PP
 .BR watch
 will run until interrupted.
@@ -84,4 +89,5 @@
 .B watch
 was written by Tony Rems <[EMAIL PROTECTED]> in 1991, with mods and
 corrections by Francois Pinard.  It was reworked and new features added by
-Mike Coleman <[EMAIL PROTECTED]> in 1999.
+Mike Coleman <[EMAIL PROTECTED]> in 1999.  The beep and error handling 
features were
+added by Morty Abzug <[EMAIL PROTECTED]> in 2008.
diff -Nur procps-3.2.7.orig/watch.c procps-3.2.7/watch.c
--- procps-3.2.7.orig/watch.c   2006-06-17 05:18:38.000000000 -0400
+++ procps-3.2.7/watch.c        2008-08-08 16:35:16.000000000 -0400
@@ -8,6 +8,7 @@
  * Mike Coleman <[EMAIL PROTECTED]>.
  *
  * Changes by Albert Cahalan, 2002-2003.
+ * stderr handling and beep option added by Morty Abzug, 2008
  */
 
 #define VERSION "0.2.0"
@@ -35,13 +36,14 @@
        {"differences", optional_argument, 0, 'd'},
        {"help", no_argument, 0, 'h'},
        {"interval", required_argument, 0, 'n'},
+       {"beep", no_argument, 0, 'b'},
        {"no-title", no_argument, 0, 't'},
        {"version", no_argument, 0, 'v'},
        {0, 0, 0, 0}
 };
 
 static char usage[] =
-    "Usage: %s [-dhntv] [--differences[=cumulative]] [--help] [--interval=<n>] 
[--no-title] [--version] <command>\n";
+    "Usage: %s [-bdhntv] [--beep] [--differences[=cumulative]] [--help] 
[--interval=<n>] [--no-title] [--version] <command>\n";
 
 static char *progname;
 
@@ -140,17 +142,24 @@
        int optc;
        int option_differences = 0,
            option_differences_cumulative = 0,
+                       option_beep = 0,
            option_help = 0, option_version = 0;
        double interval = 2;
        char *command;
        int command_length = 0; /* not including final \0 */
+       int pipefd[2];
+       int status;
+       pid_t child;
 
        setlocale(LC_ALL, "");
        progname = argv[0];
 
-       while ((optc = getopt_long(argc, argv, "+d::hn:vt", longopts, (int *) 
0))
+       while ((optc = getopt_long(argc, argv, "+bd::hn:vt", longopts, (int *) 
0))
               != EOF) {
                switch (optc) {
+               case 'b':
+                       option_beep = 1;
+                       break;
                case 'd':
                        option_differences = 1;
                        if (optarg)
@@ -191,6 +200,7 @@
 
        if (option_help) {
                fprintf(stderr, usage, progname);
+               fputs("  -b, --beep\t\t\t\tbeep if the command has a non-zero 
exit\n", stderr);
                fputs("  -d, --differences[=cumulative]\thighlight changes 
between updates\n", stderr);
                fputs("\t\t(cumulative means highlighting is cumulative)\n", 
stderr);
                fputs("  -h, --help\t\t\t\tprint a summary of the options\n", 
stderr);
@@ -261,11 +271,46 @@
                        free(header);
                }
 
-               if (!(p = popen(command, "r"))) {
-                       perror("popen");
+               /* allocate pipes */
+               if (pipe(pipefd)<0) {
+                 perror("pipe");
+                       do_exit(7);
+         }
+
+               /* flush stdout and stderr, since we're about to do fd stuff */
+               fflush(stdout);
+               fflush(stderr);
+
+               /* fork to prepare to run command */
+               child=fork();
+
+               if (child<0) { /* fork error */
+                 perror("fork");
                        do_exit(2);
+               } else if (child==0) { /* in child */
+                       close (pipefd[0]); /* child doesn't need read side of 
pipe */
+                       close (1); /* prepare to replace stdout with pipe */
+                       if (dup2 (pipefd[1], 1)<0) { /* replace stdout with 
write side of pipe */
+                         perror("dup2");
+                               exit(3);
+                       }
+                       dup2(1, 2); /* stderr should default to stdout */
+                       status=system(command); /* watch manpage promises 
quoting, so no execvp */
+                       if (!WIFEXITED(status)) { /* child exits nonzero if 
command does */
+                         exit(1);
+                       } else {
+                         exit(WEXITSTATUS(status));
+                 }
                }
 
+               /* otherwise, we're in parent */
+               close(pipefd[1]); /* close write side of pipe */
+               if ((p=fdopen(pipefd[0], "r"))==NULL) {
+                 perror("fdopen");
+                       do_exit(5);
+               }
+
+
                for (y = show_title; y < height; y++) {
                        int eolseen = 0, tabpending = 0;
                        for (x = 0; x < width; x++) {
@@ -313,7 +358,18 @@
                        oldeolseen = eolseen;
                }
 
-               pclose(p);
+               fclose(p);
+
+               /* harvest child process and get status, propagated from 
command */
+               if (waitpid(child, &status, 0)<0) {
+                 perror("waitpid");
+                       do_exit(8);
+               };
+
+               /* if child process exited in error, beep if option_beep is set 
*/
+               if (option_beep && (!WIFEXITED(status) || WEXITSTATUS(status))) 
{
+                 beep();
+               }
 
                first_screen = 0;
                refresh();

Reply via email to