Date: Fri, 22 Oct 2004 02:33:02 +0300 From: Peter Pentchev <roam@ringlet.net> To: Robert Watson <rwatson@FreeBSD.org> Cc: freebsd-hackers@FreeBSD.org Subject: Re: [CFR] Specify the lock(1) timeout unit Message-ID: <20041021233302.GE7732@straylight.m.ringlet.net> In-Reply-To: <Pine.NEB.3.96L.1041021163636.10079R-100000@fledge.watson.org> References: <20041021113709.GB7732@straylight.m.ringlet.net> <Pine.NEB.3.96L.1041021163636.10079R-100000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--wTWi5aaYRw9ix9vO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Thu, Oct 21, 2004 at 04:38:10PM -0400, Robert Watson wrote:
> On Thu, 21 Oct 2004, Peter Pentchev wrote:
>=20
> > 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=
=2E=20
> > I could commit this in a week if there are no objections.=20
>=20
> 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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
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 -0000 1.11
+++ src/usr.bin/lock/lock.1 21 Oct 2004 23:05:25 -0000
@@ -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=20
+.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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
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 -0000 1.18
+++ src/usr.bin/lock/lock.c 21 Oct 2004 23:06:59 -0000
@@ -85,6 +85,20 @@
long nexttime; /* keep the timeout time */
int no_timeout; /* lock terminal forever */
int vtyunlock; /* Unlock flag and code. */
+const char *timeout_str =3D "minute";
+int timeout_mul =3D 60;
+
+struct timeout_spec {
+ char spec;
+ int mul;
+ const char *str;
+} timeout_spec[] =3D {
+ {'s', 1, "second"},
+ {'m', 60, "minute"},
+ {'h', 3600, "hour"},
+ {'d', 86400, "day"},
+ {'\0', 0, NULL},
+};
=20
/*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;
=20
openlog("lock", LOG_ODELAY, LOG_AUTH);
=20
@@ -109,8 +124,19 @@
while ((ch =3D getopt(argc, argv, "npt:v")) !=3D -1)
switch((char)ch) {
case 't':
- if ((sectimeout =3D atoi(optarg)) <=3D 0)
+ if ((sectimeout =3D strtol(optarg, &endarg, 10)) <=3D 0)
errx(1, "illegal timeout value");
+ if (*endarg =3D=3D '\0')
+ break;
+ if (endarg[1] !=3D '\0')
+ errx(1, "illegal timeout specifier");
+ for (ts =3D timeout_spec; ts->spec !=3D '\0'; ts++)
+ if (ts->spec =3D=3D *endarg)
+ break;
+ if (ts->spec =3D=3D '\0')
+ errx(1, "illegal timeout specifier");
+ timeout_mul =3D ts->mul;
+ timeout_str =3D ts->str;
break;
case 'p':
usemine =3D 1;
@@ -128,7 +154,7 @@
default:
usage();
}
- timeout.tv_sec =3D sectimeout * 60;
+ timeout.tv_sec =3D sectimeout * timeout_mul;
=20
setuid(getuid()); /* discard privs */
=20
@@ -139,7 +165,7 @@
errx(1, "not a terminal?");
if (gettimeofday(&timval, (struct timezone *)NULL))
err(1, "gettimeofday");
- nexttime =3D timval.tv_sec + (sectimeout * 60);
+ nexttime =3D timval.tv_sec + (sectimeout * timeout_mul);
timval_sec =3D timval.tv_sec;
timp =3D localtime(&timval_sec);
ap =3D asctime(timp);
@@ -200,8 +226,8 @@
if (no_timeout)
(void)printf(" no timeout.");
else
- (void)printf(" timeout in %d minute%s.", sectimeout,
- sectimeout !=3D 1 ? "s" : "");
+ (void)printf(" timeout in %d %s%s.", sectimeout,
+ timeout_str, sectimeout !=3D 1 ? "s" : "");
if (vtylock)
(void)printf(" vty locked.");
(void)printf("\ntime now is %.20s%s%s", ap, tzn, ap + 19);
--=20
Peter Pentchev roam@ringlet.net roam@cnsys.bg roam@FreeBSD.org
PGP key: http://people.FreeBSD.org/~roam/roam.key.asc
Key fingerprint FDBA FD79 C26F 3C51 C95E DF9E ED18 B68D 1619 4553
The rest of this sentence is written in Thailand, on
--wTWi5aaYRw9ix9vO
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (FreeBSD)
iD8DBQFBeEcu7Ri2jRYZRVMRAr+BAKClCigdWoW5uKvZgTFXQOCSVOs9ZQCeIZOJ
RHCu3hldpolkQ8wvL+QIkzA=
=mTVV
-----END PGP SIGNATURE-----
--wTWi5aaYRw9ix9vO--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20041021233302.GE7732>