Re: [CFR] Specify the lock(1) timeout unit
On 2004.10.21 14:37:10 +0300, Peter Pentchev wrote: Here's a little patch that teaches lock(1) about timeouts specified in seconds, hours, or days in addition to the minutes it currently assumes. I could commit this in a week if there are no objections. Wouldn't it be more natural to just append the time-unit type to the argument given to -t, e.g. -t 10s or -t 10h. That just seem like the more intuitive way to handle it to me... Note: this is a suggestion, not an objection to the original patch. -- Simon L. Nielsen FreeBSD Documentation Team pgplLx6xiPz8q.pgp Description: PGP signature
Re: [CFR] Specify the lock(1) timeout unit
On Thu, Oct 21, 2004 at 04:32:08PM +0200, Simon L. Nielsen wrote: On 2004.10.21 14:37:10 +0300, Peter Pentchev wrote: Here's a little patch that teaches lock(1) about timeouts specified in seconds, hours, or days in addition to the minutes it currently assumes. I could commit this in a week if there are no objections. Wouldn't it be more natural to just append the time-unit type to the argument given to -t, e.g. -t 10s or -t 10h. That just seem like the more intuitive way to handle it to me... Note: this is a suggestion, not an objection to the original patch. Yes, please. :-) Cheers, -- Ruslan Ermilov [EMAIL PROTECTED] FreeBSD committer pgp1k9I6rgODg.pgp Description: PGP signature
Re: [CFR] Specify the lock(1) timeout unit
On Thu, 21 Oct 2004, Peter Pentchev wrote: Here's a little patch that teaches lock(1) about timeouts specified in seconds, hours, or days in addition to the minutes it currently assumes. I could commit this in a week if there are no objections. I think the normal convention here (see also shutdown(8), etc) is to have the unit be specified as part of the time specification rather than as a separate argument. I.e., lock -t 5m rather than lock -t 5 -u m. If we don't already have it, maybe we need humanize and dehumanize functions for time as well as disk storage? Robert N M Watson FreeBSD Core Team, TrustedBSD Projects [EMAIL PROTECTED] Principal Research Scientist, McAfee Research G'luck, Peter Index: src/usr.bin/lock/lock.1 === RCS file: /home/ncvs/src/usr.bin/lock/lock.1,v retrieving revision 1.11 diff -u -r1.11 lock.1 --- src/usr.bin/lock/lock.1 2 Jul 2004 22:22:27 - 1.11 +++ src/usr.bin/lock/lock.1 21 Oct 2004 10:39:13 - @@ -42,6 +42,7 @@ .Nm .Op Fl npv .Op Fl t Ar timeout +.Op Fl u Ar unit .Sh DESCRIPTION The .Nm @@ -63,7 +64,22 @@ .It Fl t Ar timeout The time limit (default 15 minutes) is changed to .Ar timeout -minutes. +minutes, or units specified by the +.Fl u +option. +.It Fl u Ar unit +Specify the time measurement unit for the time limit. +The +.Ar unit +argument may be one of +.Sq s +for seconds, +.Sq m +for minutes (the default), +.Sq h +for hours, or +.Sq d +for days. .It Fl v Disable switching virtual terminals while this terminal is locked. This option is implemented in a way similar to the Index: src/usr.bin/lock/lock.c === RCS file: /home/ncvs/src/usr.bin/lock/lock.c,v retrieving revision 1.18 diff -u -r1.18 lock.c --- src/usr.bin/lock/lock.c 22 Jan 2004 04:24:15 - 1.18 +++ src/usr.bin/lock/lock.c 21 Oct 2004 11:07:36 - @@ -85,6 +85,20 @@ long nexttime; /* keep the timeout time */ intno_timeout; /* lock terminal forever */ int vtyunlock; /* Unlock flag and code. */ +const char *timeout_str = minute; +int timeout_mul = 60; + +struct timeout_spec { + char spec; + int mul; + const char *str; +} timeout_spec[] = { + {'s', 1, second}, + {'m',60, minute}, + {'h', 3600, hour}, + {'d', 86400, day}, + {'\0',0, NULL}, +}; /*ARGSUSED*/ int @@ -98,20 +112,31 @@ int ch, failures, sectimeout, usemine, vtylock; char *ap, *mypw, *ttynam, *tzn; char hostname[MAXHOSTNAMELEN], s[BUFSIZ], s1[BUFSIZ]; + struct timeout_spec *ts; openlog(lock, LOG_ODELAY, LOG_AUTH); sectimeout = TIMEOUT; + timeout_mul = 60; mypw = NULL; usemine = 0; no_timeout = 0; vtylock = 0; - while ((ch = getopt(argc, argv, npt:v)) != -1) + while ((ch = getopt(argc, argv, npt:u:v)) != -1) switch((char)ch) { case 't': if ((sectimeout = atoi(optarg)) = 0) errx(1, illegal timeout value); break; + case 'u': + for (ts = timeout_spec; ts-spec != '\0'; ts++) + if (ts-spec == optarg[0]) + break; + if (ts-spec == '\0') + errx(1, illegal timeout unit specifier); + timeout_mul = ts-mul; + timeout_str = ts-str; + break; case 'p': usemine = 1; if (!(pw = getpwuid(getuid( @@ -128,7 +153,7 @@ default: usage(); } - timeout.tv_sec = sectimeout * 60; + timeout.tv_sec = sectimeout * timeout_mul; setuid(getuid()); /* discard privs */ @@ -139,7 +164,7 @@ errx(1, not a terminal?); if (gettimeofday(timval, (struct timezone *)NULL)) err(1, gettimeofday); - nexttime = timval.tv_sec + (sectimeout * 60); + nexttime = timval.tv_sec + (sectimeout * timeout_mul); timval_sec = timval.tv_sec; timp = localtime(timval_sec); ap = asctime(timp); @@ -200,8 +225,8 @@ if (no_timeout) (void)printf( no timeout.); else - (void)printf( timeout in %d minute%s., sectimeout, - sectimeout != 1 ? s : ); + (void)printf( timeout in %d %s%s., sectimeout, + timeout_str, sectimeout != 1 ? s : ); if (vtylock) (void)printf( vty locked.); (void)printf(\ntime now is %.20s%s%s, ap, tzn, ap + 19); @@ -243,7 +268,7 @@
Re: [CFR] Specify the lock(1) timeout unit
On Thu, Oct 21, 2004 at 04:38:10PM -0400, Robert Watson wrote: On Thu, 21 Oct 2004, Peter Pentchev wrote: Here's a little patch that teaches lock(1) about timeouts specified in seconds, hours, or days in addition to the minutes it currently assumes. I could commit this in a week if there are no objections. I think the normal convention here (see also shutdown(8), etc) is to have the unit be specified as part of the time specification rather than as a separate argument. I.e., lock -t 5m rather than lock -t 5 -u m. If we don't already have it, maybe we need humanize and dehumanize functions for time as well as disk storage? [randomly picking this message to reply to, since the others voiced the same concern and suggestion] Yep, on second thought it only seems natural to append the unit to the time specification; guess I wasn't thinking straight earlier today when I whipped this up in a hurry... Thanks everyone for pointing out the blindingly obvious - in this case, it *was* needed! :) Attached is an almost trivial patch that implements this, parsing things like '10s', '2m', '15h', or '2d' just as the previous version did - seconds, minutes, hours, days. As to factoring out the time specification parsing, it may not be just as easy as with disk storage units. The main problem here is that the utilites that parse time specifiers do so for a variety of different reasons, and then use the result in different ways. The secondary problem, maybe even more severe, is that the different utilities parse very different time formats, e.g. date(1) pretty much only understands +/-num[ymwdHMS], shutdown(8) takes either +minutes or a full or partial [[[yy]mm]dd]hhmmss specification, and find(1) and cvs(1) use the GNU getdate.y thing which is... well, let me say 'hairy' lest I use a stronger word. (As a side note, wow, I never knew that find(1) could do 'last year', 'a fortnight ago', or '22:00 IDLW'; makes sense, though, since they use the same code.) Sooo.. if we should create a unified time parsing function, what should it parse - an interval, an absolute time, or what? That is, what should it *return* - an interval in seconds, or the absolute time (time_t or struct tm) that the input specifies either directly or as an offset from the current time, both, neither, or the air velocity of an unladen swallow? How should it deal with nonexistent times - return an error, try to round them up, try to round them down, or silently convert them to the Antartica/South_Pole zone and snicker behind the curtain? How should it deal with month specifications? (gee, I hope no one tries to use a month unit as a lock(1) timeout, but you never know with some people) Anyway, here's the lock(1) patch that lets it handle 's', 'm', 'h', or 'd' appended to the timeout value. G'luck, Peter Index: src/usr.bin/lock/lock.1 === RCS file: /home/ncvs/src/usr.bin/lock/lock.1,v retrieving revision 1.11 diff -u -r1.11 lock.1 --- src/usr.bin/lock/lock.1 2 Jul 2004 22:22:27 - 1.11 +++ src/usr.bin/lock/lock.1 21 Oct 2004 23:05:25 - @@ -64,6 +64,21 @@ The time limit (default 15 minutes) is changed to .Ar timeout minutes. +The timeout value may optionally be followed by a time unit specifier, +one of +.Sq s +for seconds, +.Sq m +for minutes (the default), +.Sq h +for hours, or +.Sq d +for days. +Thus, +.Sq -t 2 +would specify a timeout of two minutes, while +.Sq -t 10s +would specify a timeout of ten seconds. .It Fl v Disable switching virtual terminals while this terminal is locked. This option is implemented in a way similar to the Index: src/usr.bin/lock/lock.c === RCS file: /home/ncvs/src/usr.bin/lock/lock.c,v retrieving revision 1.18 diff -u -r1.18 lock.c --- src/usr.bin/lock/lock.c 22 Jan 2004 04:24:15 - 1.18 +++ src/usr.bin/lock/lock.c 21 Oct 2004 23:06:59 - @@ -85,6 +85,20 @@ long nexttime; /* keep the timeout time */ intno_timeout; /* lock terminal forever */ intvtyunlock; /* Unlock flag and code. */ +const char *timeout_str = minute; +inttimeout_mul = 60; + +struct timeout_spec { + char spec; + int mul; + const char *str; +} timeout_spec[] = { + {'s', 1, second}, + {'m',60, minute}, + {'h', 3600, hour}, + {'d', 86400, day}, + {'\0',0, NULL}, +}; /*ARGSUSED*/ int @@ -96,8 +110,9 @@ struct itimerval ntimer, otimer; struct tm *timp; int ch, failures, sectimeout, usemine, vtylock; - char *ap, *mypw, *ttynam, *tzn; + char *ap, *mypw, *ttynam, *tzn, *endarg; char hostname[MAXHOSTNAMELEN], s[BUFSIZ], s1[BUFSIZ]; + struct timeout_spec *ts; openlog(lock, LOG_ODELAY, LOG_AUTH); @@ -109,8 +124,19 @@