Skip to content

Commit

Permalink
11460 logadm docmd() can't handle too much on stderr
Browse files Browse the repository at this point in the history 
Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Jason King <jason.king@joyent.com>
Reviewed by: Andy Fiddaman <omnios@citrus-it.co.uk>
Approved by: Robert Mustacchi <rm@joyent.com>
  • Loading branch information
@rmustacc
Mike Gerdts authored and rmustacc committed Jul 19, 2019
1 parent 1418cdc commit d5dace5
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 37 deletions.
76 changes: 50 additions & 26 deletions usr/src/cmd/logadm/main.c
View file
Expand Up @@ -20,7 +20,7 @@
*/
/*
* Copyright (c) 2001, 2010, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, Joyent, Inc. All rights reserved.
* Copyright 2019 Joyent, Inc.
*
* logadm/main.c -- main routines for logadm
*
Expand All @@ -41,6 +41,8 @@
#include <sys/sysmacros.h>
#include <time.h>
#include <utime.h>
#include <poll.h>
#include <errno.h>
#include "err.h"
#include "lut.h"
#include "fn.h"
Expand Down Expand Up @@ -1030,16 +1032,11 @@ docmd(struct opts *opts, const char *msg, const char *cmd,
return; /* -n means don't really do it */

/*
* run the cmd and see if it failed. this function is *not* a
* generic command runner -- we depend on some knowledge we
* have about the commands we run. first of all, we expect
* errors to spew something to stderr, and that something is
* typically short enough to fit into a pipe so we can wait()
* for the command to complete and then fetch the error text
* from the pipe. we also expect the exit codes to make sense.
* notice also that we only allow a command name which is an
* absolute pathname, and two args must be supplied (the
* second may be NULL, or they may both be NULL).
* Run the cmd and see if it failed. This function is *not* a generic
* command runner. The command name must be an absolute pathname, and
* two args must be supplied (the second may be NULL, or they may both
* be NULL). Any output (stdout and stderr) from the child process is
* logged to stderr and perhaps sent to an email recipient.
*/
if (pipe(errpipe) < 0)
err(EF_SYS, "pipe");
Expand All @@ -1048,25 +1045,51 @@ docmd(struct opts *opts, const char *msg, const char *cmd,
err(EF_SYS, "fork");
else if (pid) {
int wstat;
int count;
struct pollfd pfd;
boolean_t first = B_TRUE;

/* parent */
(void) close(errpipe[1]);
if (waitpid(pid, &wstat, 0) < 0)

pfd.fd = errpipe[0];
pfd.events = POLLIN | POLLRDNORM | POLLRDBAND | POLLPRI;
for (;;) {

pfd.revents = 0;
if (poll(&pfd, 1, -1) == -1) {
if (errno == EINTR) {
continue;
}
err(EF_SYS, "poll");
break;
}
if ((pfd.events & pfd.revents) != 0) {
if (first) {
err(EF_WARN,
"command failed: %s%s%s%s%s%s%s",
cmd,
(arg1) ? " " : "",
(arg1) ? arg1 : "",
(arg2) ? " " : "",
(arg2) ? arg2 : "",
(arg3) ? " " : "",
(arg3) ? arg3 : "");
first = B_FALSE;
}
err_fromfd(pfd.fd);
}
if ((pfd.revents & (POLLERR | POLLHUP)) != 0) {
break;
}
}
if (waitpid(pid, &wstat, 0) < 0) {
err(EF_SYS, "waitpid");
return;
}

/* check for stderr output */
if (ioctl(errpipe[0], FIONREAD, &count) >= 0 && count) {
err(EF_WARN, "command failed: %s%s%s%s%s%s%s",
cmd,
(arg1) ? " " : "",
(arg1) ? arg1 : "",
(arg2) ? " " : "",
(arg2) ? arg2 : "",
(arg3) ? " " : "",
(arg3) ? arg3 : "");
err_fromfd(errpipe[0]);
} else if (WIFSIGNALED(wstat))
if (!first) {
/* Assume the command gave a useful error */
} else if (WIFSIGNALED(wstat)) {
err(EF_WARN,
"command died, signal %d: %s%s%s%s%s%s%s",
WTERMSIG(wstat),
Expand All @@ -1077,7 +1100,7 @@ docmd(struct opts *opts, const char *msg, const char *cmd,
(arg2) ? arg2 : "",
(arg3) ? " " : "",
(arg3) ? arg3 : "");
else if (WIFEXITED(wstat) && WEXITSTATUS(wstat))
} else if (WIFEXITED(wstat) && WEXITSTATUS(wstat)) {
err(EF_WARN,
"command error, exit %d: %s%s%s%s%s%s%s",
WEXITSTATUS(wstat),
Expand All @@ -1088,6 +1111,7 @@ docmd(struct opts *opts, const char *msg, const char *cmd,
(arg2) ? arg2 : "",
(arg3) ? " " : "",
(arg3) ? arg3 : "");
}

(void) close(errpipe[0]);
} else {
Expand Down
57 changes: 46 additions & 11 deletions usr/src/cmd/logadm/tester
View file
Expand Up @@ -21,7 +21,7 @@
#
#
# Copyright (c) 2001, 2010, Oracle and/or its affiliates. All rights reserved.
# Copyright (c) 2013, Joyent, Inc. All rights reserved.
# Copyright 2019 Joyent, Inc.
#
#
# tester - run logadm tests
Expand All @@ -31,19 +31,19 @@
# logadm itself).
#
# to run all the tests:
# tester [-f] <bindir>
# tester [-f] <bindir>
#
# to run just a few tests, given their names:
# tester [-f] <bindir> globtest1 luttest1
# tester [-f] <bindir> globtest1 luttest1
#
# to setup a test and stop so you can run it by hand:
# tester [-f] -s globtest1 <bindir>
# tester [-f] -s globtest1 <bindir>
#
# tester will tell you what tmp directory it created for
# the test. to run it, cd there and run:
# sh runtest
# to check the results, run:
# sh checktest
# tester will tell you what tmp directory it created for
# the test. to run it, cd there and run:
# sh runtest
# to check the results, run:
# sh checktest
#
# -f means "fast" -- without it, watchmalloc(3MALLOC) is setup for
# each test and they run a zillion times slower and produce core
Expand Down Expand Up @@ -92,7 +92,8 @@ umask 002;
"logadm20",
"logadm21",
"logadm22",
"logadm23"
"logadm23",
"stderr1"
);

use Getopt::Std;
Expand Down Expand Up @@ -275,7 +276,7 @@ sub set_testconffile {
# Default settings for system log file management.
# The -w option to logadm(1M) is the preferred way to write to this file,
# but if you do edit it by hand, use "logadm -V" to check it for errors.
#
#
# The format of lines in this file is:
# <logname> <options>
# For each logname listed here, the default options to logadm
Expand Down Expand Up @@ -2214,3 +2215,37 @@ exit 0
EOF
}
###########################################################################
#
# stderr1 -- ensure verbose stderr does not deadlock
#
###########################################################################
sub stderr1 {
set_file('logfile', 'initially logfile');
set_file('std.err.uniq.expect', <<'EOF');
1 logadm: Warning: command failed: /bin/sh -c exec 1>&2; for i in {1..250000}; do echo pre-command-stuff; done
250000 pre-command-stuff
1 logadm: Warning: command failed: /bin/sh -c exec 1>&2; for i in {1..250000}; do echo post-command-stuff; done
250000 post-command-stuff
EOF
set_file('checktest', <<'EOF');
[ -s std.out ] && exit 1
/bin/diff -u std.err.uniq.expect std.err.uniq || exit 1
exit 0
EOF
# The output redirection below looks wrong, but it is not. The redirect
# of stderr before the redirect of stdout causes stderr to be piped to
# uniq.
set_file('runtest', <<"EOF");
# test "stderr1"
$envsetup
exec $bindir/logadm -f /dev/null -p now logfile \\
-b 'exec 1>&2; for i in {1..250000}; do echo pre-command-stuff; done' \\
-a 'exec 1>&2; for i in {1..250000}; do echo post-command-stuff; done' \\
2>&1 >std.out | uniq -c >std.err.uniq
EOF
}

0 comments on commit d5dace5

Please sign in to comment.