From owner-svn-src-all@FreeBSD.ORG Wed Jul 16 11:31:36 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id C47DC8A0; Wed, 16 Jul 2014 11:31:36 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4BAFE2D17; Wed, 16 Jul 2014 11:31:36 +0000 (UTC) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id s6GBVSfP050330 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 16 Jul 2014 14:31:28 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.8.3 kib.kiev.ua s6GBVSfP050330 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id s6GBVSeM050329; Wed, 16 Jul 2014 14:31:28 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 16 Jul 2014 14:31:28 +0300 From: Konstantin Belousov To: Baptiste Daroussin Subject: Re: svn commit: r268745 - in head/usr.bin: . timeout Message-ID: <20140716113128.GV93733@kib.kiev.ua> References: <201407160955.s6G9taro084054@svn.freebsd.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="LhGscpP45fjEbabV" Content-Disposition: inline In-Reply-To: <201407160955.s6G9taro084054@svn.freebsd.org> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Jul 2014 11:31:36 -0000 --LhGscpP45fjEbabV Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 16, 2014 at 09:55:36AM +0000, Baptiste Daroussin wrote: > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include Is this header needed ? The headers are unordered. > +#include > +#include > + > +#define EXIT_TIMEOUT 124 > + > +static sig_atomic_t sig_chld =3D 0; > +static sig_atomic_t sig_term =3D 0; > +static sig_atomic_t sig_alrm =3D 0; > +static sig_atomic_t sig_ign =3D 0; > + > +static void > +usage(void) > +{ There should be empty line after '{' if no local vars are declared. > + fprintf(stderr, "Usage: %s [--signal sig | -s sig] [--preserve-status]" > + " [--kill-after time | -k time] [--foreground] " > + " \n", getprogname()); > + > + exit(EX_USAGE); > +} > + > +static double > +parse_duration(const char *duration) > +{ > + double ret; > + char *end; > + > + ret =3D strtod(duration, &end); > + if (ret =3D=3D 0 && end =3D=3D duration) > + errx(EXIT_FAILURE, "invalid duration"); > + > + if (end =3D=3D NULL || *end =3D=3D '\0') > + return (ret); > + > + if (end !=3D NULL && *(end + 1) !=3D '\0') > + errx(EX_USAGE, "invalid duration"); > + > + switch (*end) { > + case 's': > + break; > + case 'm': > + ret *=3D 60; > + break; > + case 'h': > + ret *=3D 60 * 60; > + break; > + case 'd': > + ret *=3D 60 * 60 * 24; > + break; > + default: > + errx(EX_USAGE, "invalid duration"); > + } > +=09 > + if (ret < 0 || ret >=3D 100000000UL) > + errx(EX_USAGE, "invalid duration"); > + > + return (ret); > +} > + > +static int > +parse_signal(const char *str) > +{ > + int sig, i; > + const char *err; > + > + sig =3D strtonum(str, 0, sys_nsig, &err); > + > + if (err =3D=3D NULL) > + return (sig); > + if (strncasecmp(str, "SIG", 3) =3D=3D 0) > + str +=3D 3; > + > + for (i =3D 1; i < sys_nsig; i++) { > + if (strcasecmp(str, sys_signame[i]) =3D=3D 0) > + return (i); > + } > +=09 > + errx(EX_USAGE, "invalid signal"); > +} > + > +static void > +sig_handler(int signo) > +{ > + if (sig_ign !=3D 0 && signo =3D=3D sig_ign) { > + sig_ign =3D 0; > + return; > + } > + > + switch(signo) { > + case 0: > + case SIGINT: > + case SIGHUP: > + case SIGQUIT: > + case SIGTERM: > + sig_term =3D signo; > + break; > + case SIGCHLD: > + sig_chld =3D 1; > + break; > + case SIGALRM: > + sig_alrm =3D 1; > + break; > + } > +} > + > +static void > +set_interval(double iv) > +{ > + struct itimerval tim; > + > + memset(&tim, 0, sizeof(tim)); > + tim.it_value.tv_sec =3D (time_t)iv; > + iv -=3D (time_t)iv; > + tim.it_value.tv_usec =3D (suseconds_t)(iv * 1000000UL); > + > + if (setitimer(ITIMER_REAL, &tim, NULL) =3D=3D -1) > + err(EX_OSERR, "setitimer()"); > +} > + > +int > +main(int argc, char **argv) > +{ > + int ch; > + unsigned long i; > + int foreground, preserve; > + int error, pstat, status; > + int killsig =3D SIGTERM; > + int killedwith; > + pid_t pgid, pid, cpid; > + double first_kill; > + double second_kill; > + bool timedout =3D false; > + bool do_second_kill =3D false; > + struct sigaction signals; > + int signums[] =3D { > + -1, > + SIGTERM, > + SIGINT, > + SIGHUP, > + SIGCHLD, > + SIGALRM, > + SIGQUIT, > + }; > + > + foreground =3D preserve =3D 0; > + second_kill =3D 0; > + cpid =3D -1; > + > + struct option longopts[] =3D { This should be static const. > + { "preserve-status", no_argument, &preserve, 1 }, > + { "foreground", no_argument, &foreground, 1 }, > + { "kill-after", required_argument, NULL, 'k'}, > + { "signal", required_argument, NULL, 's'}, > + { "help", no_argument, NULL, 'h'}, > + { NULL, 0, NULL, 0 } > + }; > + > + while ((ch =3D getopt_long(argc, argv, "+k:s:h", longopts, NULL)) !=3D = -1) { > + switch (ch) { > + case 'k': > + do_second_kill =3D true; > + second_kill =3D parse_duration(optarg); > + break; > + case 's': > + killsig =3D parse_signal(optarg); > + break; > + case 0: > + break; > + case 'h': > + default: > + usage(); > + break; > + } > + } > + > + argc -=3D optind; > + argv +=3D optind; > + > + if (argc < 2) > + usage(); > + > + first_kill =3D parse_duration(argv[0]); > + argc--; > + argv++; > + > + if (!foreground) { > + pgid =3D setpgid(0,0); > + > + if (pgid =3D=3D -1) > + err(EX_OSERR, "setpgid()"); > + } > + > + memset(&signals, 0, sizeof(signals)); > + sigemptyset(&signals.sa_mask); > + > + if (killsig !=3D SIGKILL && killsig !=3D SIGSTOP) > + signums[0] =3D killsig; > + > + for (i =3D 0; i < sizeof(signums) / sizeof(signums[0]); i ++) > + sigaddset(&signals.sa_mask, signums[i]); Calling sigaddset(..., -1); relies both on the quality of implementation, and on absense of the implementation extensions, which could define unexpected semantic for the undefined call. > + > + signals.sa_handler =3D sig_handler; > + signals.sa_flags =3D SA_RESTART; > + > + for (i =3D 0; i < sizeof(signums) / sizeof(signums[0]); i ++) > + if (signums[i] !=3D -1 && signums[i] !=3D 0 && > + sigaction(signums[i], &signals, NULL) =3D=3D -1) > + err(EX_OSERR, "sigaction()"); > + > + signal(SIGTTIN, SIG_IGN); > + signal(SIGTTOU, SIG_IGN); > + > + pid =3D fork(); > + if (pid =3D=3D -1) > + err(EX_OSERR, "fork()"); > + else if (pid =3D=3D 0) { > + /* child process */ > + signal(SIGTTIN, SIG_DFL); > + signal(SIGTTOU, SIG_DFL); Shouldn't you preserve the signal disposition for all intercepted signals and restore it for the child ? > + > + error =3D execvp(argv[0], argv); > + if (error =3D=3D -1) > + err(EX_UNAVAILABLE, "exec()"); > + } > + > + if (sigprocmask(SIG_BLOCK, &signals.sa_mask, NULL) =3D=3D -1) > + err(EX_OSERR, "sigprocmask()"); > + > + /* parent continues here */ > + set_interval(first_kill); > + > + sigemptyset(&signals.sa_mask); This statement is not needed, you repeat it inside for() loop below. > + > + for (;;) { > + killedwith =3D killsig; > + sigemptyset(&signals.sa_mask); > + sigsuspend(&signals.sa_mask); > + > + if (sig_chld) { > + sig_chld =3D 0; > + while (((cpid =3D wait(&status)) < 0) && errno !=3D EINTR) > + continue; Should the test be errno =3D=3D EINTR instead ? > + > + if (cpid =3D=3D pid) { > + pstat =3D status; > + break; > + } > + } else if (sig_alrm) { > + sig_alrm =3D 0; > + > + timedout =3D true; > + if (!foreground) > + killpg(pgid, killsig); > + else > + kill(pid, killsig); > + > + if (do_second_kill) { > + set_interval(second_kill); > + second_kill =3D 0; > + sig_ign =3D killsig; > + killsig =3D SIGKILL; > + } else > + break; > + > + } else if (sig_term) { > + if (!foreground) > + killpg(pgid, killsig); > + else > + kill(pid, sig_term); > + > + if (do_second_kill) { > + set_interval(second_kill); > + second_kill =3D 0; > + sig_ign =3D killsig; > + killsig =3D SIGKILL; > + } else > + break; > + } > + } > + > + while (cpid !=3D pid && wait(&pstat) =3D=3D -1) { > + if (errno !=3D EINTR) There, the test looks correct. > + err(EX_OSERR, "waitpid()"); > + } > + > + if (WEXITSTATUS(pstat)) > + pstat =3D WEXITSTATUS(pstat); > + else if(WIFSIGNALED(pstat)) > + pstat =3D 128 + WTERMSIG(pstat); > + > + if (timedout && !preserve) > + pstat =3D EXIT_TIMEOUT; > + > + return (pstat); > +} --LhGscpP45fjEbabV Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJTxmKPAAoJEJDCuSvBvK1BQvIP/RH+KuzDEEd53symNv5dELw8 Mne4wGbjm7rEDV6KXBcHo1cf/dhDk9xWJnlskd99A6ECmwcoGXBigwpVMJJoUE4C wbz3KhEkmXoK1QiV+7S1XfFHsT1ltHZsglcL/senHN7+sP2L6hC7q00m+OBS1oOa rOFcIxnnkd/k05fLe8x/b7MQVZ39xCyaHb4gKIwa7mK9QYwFxxHbksl8WM57Q/Ey 5HjtI1vdu8mJWH5AOKgu4rlm97n0v6DQxGOFtiMCM54dJJ12Os/P57FDepMaq6h6 ZULU7wf+zhZGPZ3OXpJGSJt7KZeVViyzNO2dHLD17MvG3i7msarH4VxMXZkgk4av eIVLDzNFF4BLe9DKmdd6GQ/qyjtD7F5pU6MBxZuhT1LH3hOwFpbBbn8oL+JGlywU UMb9QGC/wbbpanVC0ZzgLpJtp7eMF9MiFEkItVIJ88OeEyfWBoYy3ZgCCbPKziGg rcA7JzMWI3TH2uNvlq9NY6zWq+I+pKI0LlRU/dGL8FOaGlSq+djNclSSL9XfEnSd VbC/kM1KTiAiO7hvr087UFWX2Uu+iHraD2ZmOC3vE/kKJROQBAPuCOGH+O2vPtBs mTOFmIpSlw+dkhP7JyWIv7TiacTfkI6fePcoK9BtVOEa57o2A/qzH4FPxZpAzJdT 61MkhlIPbie/pOAH1gWJ =X3yl -----END PGP SIGNATURE----- --LhGscpP45fjEbabV--