Thanks! Pádraig Brady <P@xxxxxxxxxxxxxx> 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! 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? 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]. Rather than '???' (or in addition to), please mark such comments with "FIXME". ... > + /* 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. -- To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
