From owner-freebsd-standards Sat Dec 29 12:35:56 2001 Delivered-To: freebsd-standards@freebsd.org Received: from espresso.q9media.com (espresso.q9media.com [216.254.138.122]) by hub.freebsd.org (Postfix) with ESMTP id DC83137B405 for ; Sat, 29 Dec 2001 12:35:44 -0800 (PST) Received: (from mike@localhost) by espresso.q9media.com (8.11.6/8.11.6) id fBTKXS944079; Sat, 29 Dec 2001 15:33:28 -0500 (EST) (envelope-from mike) Date: Sat, 29 Dec 2001 15:33:28 -0500 From: Mike Barcroft To: Joe Halpin Cc: "standards@FreeBSD.ORG" Subject: Re: at utility changes Message-ID: <20011229153328.D99161@espresso.q9media.com> References: <3C200BBA.9D26ED93@attbi.com> <20011228031537.B99161@espresso.q9media.com> <3C2DF35D.1F54BBC3@attbi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3C2DF35D.1F54BBC3@attbi.com>; from joe.halpin@attbi.com on Sat, Dec 29, 2001 at 10:46:21AM -0600 Organization: The FreeBSD Project Sender: owner-freebsd-standards@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Joe Halpin writes: > Here are a new set of changes. I think I got the style more or less > right this time, and cloned the stime_arg1() function from touch. I like this version much better. It only needs a few changes. > Index: at/at.c > =================================================================== > RCS file: /home/ncvs/src/usr.bin/at/at.c,v > retrieving revision 1.18.2.1 > diff -u -r1.18.2.1 at.c > --- at/at.c 2001/08/02 00:55:58 1.18.2.1 > +++ at/at.c 2001/12/29 16:41:19 > @@ -121,6 +121,7 @@ > static char *cwdname(void); > static void writefile(time_t runtimer, char queue); > static void list_jobs(void); > +static time_t ttime(const char *arg); > > /* Signal catching functions */ > > @@ -588,6 +589,80 @@ > } > } /* delete_jobs */ > > +#define ATOI2(ar) ((ar)[0] - '0') * 10 + ((ar)[1] - '0'); (ar) += 2; > + > +static time_t > +ttime(const char *arg) > +{ > + /* > + * This is pretty much a copy of stime_arg1() from touch.c. I changed This needs a second space after the period. > + * the return value and the argument list because it's more convenient > + * (IMO) to do everything in one place. - Joe Halpin > + */ > + struct timeval tv[2]; > + time_t now; > + struct tm *t; > + int yearset; > + char *p; > + > + if (gettimeofday(&tv[0], NULL)) > + panic("Cannot get current time"); > + > + /* Start with the current time. */ > + now = tv[0].tv_sec; > + if ((t = localtime(&now)) == NULL) > + panic("localtime"); > + /* [[CC]YY]MMDDhhmm[.SS] */ > + if ((p = strchr(arg, '.')) == NULL) > + t->tm_sec = 0; /* Seconds defaults to 0. */ > + else { > + if (strlen(p + 1) != 2) > + goto terr; > + *p++ = '\0'; > + t->tm_sec = ATOI2(p); > + } > + > + yearset = 0; > + switch(strlen(arg)) { > + case 12: /* CCYYMMDDhhmm */ > + t->tm_year = ATOI2(arg); > + t->tm_year *= 100; > + yearset = 1; > + /* FALLTHROUGH */ > + case 10: /* YYMMDDhhmm */ > + if (yearset) { > + yearset = ATOI2(arg); > + t->tm_year += yearset; > + } else { > + yearset = ATOI2(arg); > + if (yearset < 69) > + t->tm_year = yearset + 2000; > + else > + t->tm_year = yearset + 1900; > + } The else section can be reduced to something like: %%% yearset = ATOI2(arg); t->tm_year = yearset + 2000; %%% ...since it's impossible to schedule events to take place in the past. > + t->tm_year -= 1900; /* Convert to UNIX time. */ > + /* FALLTHROUGH */ > + case 8: /* MMDDhhmm */ > + t->tm_mon = ATOI2(arg); > + --t->tm_mon; /* Convert from 01-12 to 00-11 */ > + t->tm_mday = ATOI2(arg); > + t->tm_hour = ATOI2(arg); > + t->tm_min = ATOI2(arg); > + break; > + default: > + goto terr; > + } > + > + t->tm_isdst = -1; /* Figure out DST. */ > + tv[0].tv_sec = tv[1].tv_sec = mktime(t); > + if (tv[0].tv_sec != -1) > + return tv[0].tv_sec; > + else > + terr: > + panic("out of range or illegal time specification: [[CC]YY]MMDDhhmm[.SS]"); > + return -1; /* get rid of "control reaches end of non-void function" msg */ The panic and return line should probably be indented more since they apply to the else case in normal operation. Return values should be wrapped in parens. > +} > + > int > main(int argc, char **argv) > { > @@ -598,10 +673,12 @@ > > enum { ATQ, ATRM, AT, BATCH, CAT }; /* what program we want to run */ > int program = AT; /* our default program */ > - char *options = "q:f:mvldbVc"; /* default options for at */ > + char *options = "q:f:t:rmvldbVc"; /* default options for at */ > int disp_version = 0; > time_t timer; > > + timer = -1; Much better. > + Gratuitous vertical space. > RELINQUISH_PRIVS > > /* Eat any leading paths > @@ -655,6 +732,9 @@ > break; > > case 'd': > + warnx("-d is deprecated; you should use -r in the future"); > + /* fall through to 'r' */ > + case 'r': > if (program != AT) > usage(); > > @@ -662,6 +742,14 @@ > options = "V"; > break; > > + case 't': > + if (program != AT) > + usage(); > + This vertical space isn't needed either. > + options = "V"; > + timer = ttime(optarg); > + break; > + > case 'l': > if (program != AT) > usage(); > @@ -696,7 +784,7 @@ > > if (disp_version) > fprintf(stderr, "at version " VERSION "\n" > - "Bug reports to: ig25@rz.uni-karlsruhe.de (Thomas Koenig)\n"); > + "Bug reports to: ig25@rz.uni-karlsruhe.de (Thomas Koenig)\n"); I wonder if the author accepts bug reports on customized versions of his software. > > /* select our program > */ > @@ -723,7 +811,14 @@ > break; > > case AT: > - timer = parsetime(argc, argv); > + Gratuitous vertical space. > + /* > + * If timer is > -1, then the user gave the time with -t. > + * In that case, it's already been set. If not, set it now. > + */ Comments should go to 80 columns, then wrap. Also, this requires a second space after the period. > + if (timer == -1) > + timer = parsetime(argc, argv); > + > if (atverify) > { > struct tm *tm = localtime(&timer); usage() needs to be updated to reflect the new options. It's hidden away in panic.c. > Index: at/at.man > =================================================================== > RCS file: /home/ncvs/src/usr.bin/at/at.man,v > retrieving revision 1.13.2.7 > diff -u -r1.13.2.7 at.man > --- at/at.man 2001/12/14 15:53:29 1.13.2.7 > +++ at/at.man 2001/12/29 16:41:19 The SYNOPSIS sections needs to be updated to reflect the new options. > @@ -115,6 +115,8 @@ > and to run a job at 1am tomorrow, you would do > .Nm at Ar 1am tomorrow . > .Pp > +You may also use the POSIX time format (see -t argument) Periods complete sentences. > +.Pp > For both > .Nm > and > @@ -214,7 +216,7 @@ > .Nm atq . > .It Fl d > Is an alias for > -.Nm atrm . > +.Nm atrm (this option is deprecated; use -r instead) . Please add a new line between `atrm' and `(this', otherwise the whole section is sent to the macro. > .It Fl b > Is an alias for > .Nm batch . > @@ -225,6 +227,12 @@ > shows the time the job will be executed. > .It Fl c > Cat the jobs listed on the command line to standard output. > +.It Fl r > +Remove specified job(s). > +.It Fl t > +Give the job time using the POSIX time format, which is described in the `Give' should be changed to `Specify'. > +touch(1) man page. This should be changed to: %%% .Xr touch 1 manual. %%% (I prefer manual over man page, but that's an optional change.) > +Note that giving a date past 2038 may not work on 32-bit systems. This should be under the BUGS section. It also applies to the Alpha architecture, so it's probably wise to replace `32-bit' with `some'. > .El > .Sh FILES > .Bl -tag -width _ATJOB_DIR/_LOCKFILE -compact > @@ -271,4 +279,6 @@ > At was mostly written by > .An Thomas Koenig Aq ig25@rz.uni-karlsruhe.de . > The time parsing routines are by > -.An David Parsons Aq orc@pell.chi.il.us . > +.An David Parsons Aq orc@pell.chi.il.us , > +with minor enhancement by This should say either "with a minor enhancement by" or "with minor enhancements by". I prefer the latter. > +.An Joe Halpin Aq joe.halpin@attbi.com . A `STANDARDS' section should be added under `SEE ALSO': %%% .Sh STANDARDS The .Nm utility conforms with .St -p1003.1-2001 . %%% (Would you please verify this? I only glanced at the POSIX.1-2001 requirements.) Once you get those problems fixed I should be able to test and commit this for you. Best regards, Mike Barcroft To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-standards" in the body of the message