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

From: Peter Pentchev (roam_at_ringlet.net)
Date: 10/22/04

  • Next message: Ed Stover: "help with poppassd"
    Date: Fri, 22 Oct 2004 02:33:02 +0300
    To: Robert Watson <rwatson@FreeBSD.org>
    
    
    

    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 -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
    +.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 -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 = "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
    @@ -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 @@
             while ((ch = getopt(argc, argv, "npt:v")) != -1)
                     switch((char)ch) {
                     case 't':
    - if ((sectimeout = atoi(optarg)) <= 0)
    + if ((sectimeout = strtol(optarg, &endarg, 10)) <= 0)
                                     errx(1, "illegal timeout value");
    + if (*endarg == '\0')
    + break;
    + if (endarg[1] != '\0')
    + errx(1, "illegal timeout specifier");
    + for (ts = timeout_spec; ts->spec != '\0'; ts++)
    + if (ts->spec == *endarg)
    + break;
    + if (ts->spec == '\0')
    + errx(1, "illegal timeout specifier");
    + timeout_mul = ts->mul;
    + timeout_str = ts->str;
                             break;
                     case 'p':
                             usemine = 1;
    @@ -128,7 +154,7 @@
                     default:
                             usage();
                     }
    - timeout.tv_sec = sectimeout * 60;
    + timeout.tv_sec = sectimeout * timeout_mul;
     
             setuid(getuid()); /* discard privs */
     
    @@ -139,7 +165,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 +226,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);

    -- 
    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
    
    



  • Next message: Ed Stover: "help with poppassd"

    Relevant Pages

    • Re: "indefinite" wait buffer patch
      ... two years the attached diff in my releng_6 sources (patch ... The attached patch shows in fact that the wait buffer is never ... Imagine that two buffers got the timeout on swap-in simultaneously. ...
      (freebsd-hackers)
    • Re: [PATCH 18/20] x86_64: Relocatable kernel support
      ... Is it possible to make this optional (using "panic" reboot timeout)? ... There is no command line parsing at this point. ...
      (Linux-Kernel)
    • poll(2) timeout accuracy?
      ... Can anyone comment on pollimplementations regarding the timeout ... Supposing that there's nothing available to read or write, ... it possible that 'diff' below gets assigned a value that's less than ... on a 233 MHz Pentium machine. ...
      (comp.unix.programmer)