Re: [CFR] Specify the lock(1) timeout unit

2004-10-21 Thread Simon L. Nielsen
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

2004-10-21 Thread Ruslan Ermilov
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

2004-10-21 Thread Robert Watson
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

2004-10-21 Thread Peter Pentchev
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 @@