Jim Meyering wrote: > Thanks! > > Pádraig Brady <[EMAIL PROTECTED]> wrote: >> Subject: [PATCH] Add new program: timeout > ... >> +/* Given an integer value *X, and a suffix character, SUFFIX_CHAR, >> + scale *X by the multiplier implied by SUFFIX_CHAR. SUFFIX_CHAR may >> + be the NUL byte or `s' to denote seconds, `m' for minutes, `h' for >> + hours, or `d' for days. If SUFFIX_CHAR is invalid, don't modify *X >> + and return false. If *X would overflow, don't modify *X and return >> false. >> + Otherwise return true. */ >> + >> +static bool >> +apply_suffix (unsigned int *x, char suffix_char) > > You probably expect this, but I have to say it ;-) > Don't duplicate code!
of course. > apply_suffix is a tiny function, and used also by sleep.c, so let's not > duplicate it. How about putting it (static inline) in system.h instead? sleep's apply_suffic() works on floats where as timeout's work on ints. I'll see if I can merge them somewhat. Hmm I could allow float input to timeout (1.5d for example), and then round this to the nearest second later? > operand2sig is similar but not as lightweight, since it uses str2sig, > error, xstrdup and W* macros. Maybe put it in its own file in src/? > Or change xstrdup to strdup, eliminate the error call, and consider > putting it in lib/gnulib. It's probably better to go the easier > route and use a separate file in src/....[ch]. I intended but forgot to add the comment: /* FIXME: where will we refactor this to? */ I'll add a new src/ file so. > > Rather than '???' (or in addition to), please mark such > comments with "FIXME". righto >> + /* Setup handlers before fork() so that we >> + * handle any signals caused by child, without races. */ >> + signal (SIGALRM, cleanup); /* our timeout. */ >> + signal (SIGINT, cleanup); /* Ctrl-C at terminal for example. */ >> + signal (SIGQUIT, cleanup); /* Ctrl-\ at terminal for example. */ >> + signal (SIGTERM, cleanup); /* if we're killed, stop monitored process. >> */ > > Mike Frysinger's point is a good one: use signal only if absolutely > necessary. Prefer sigaction. > > Most signal-handling code in coreutils uses sigaction, and falls back on > signal ifndef SA_NOCLDSTOP. But the ifdefs and code duplication are > pretty ugly. If you're game, I'd like to try using *only* sigaction for > an initial test release, in order to see if any "reasonable portability > targets" remain that lack sigaction support. Then, if there are no > build failure reports for a long enough period, we can remove the crufty > signal-using #else blocks in a bunch of programs. Ok, I'll assume that sigaction is available. thanks, Pádraig. _______________________________________________ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
