Subject: RFC: lpr message output and exit status cleanup
To: None <tech-userlevel@netbsd.org>
From: Brian Ginsbach <ginsbach@cray.com>
List: tech-userlevel
Date: 03/25/2003 21:38:20
I'm wondering what people think of the following changes
to lpr et al. These changes were inspired by several things.
- lpr isn't consistent using stdout and stderr for messages
Most of the printf()s should be changed to warn()/warnx() calls.
(Similar to what was done in the OpenBSD version.)
- lpr doesn't exit non-zero if it can't spool one file out of many
(This was caught by a POSIX test using the lp wrapper to lpr.)
- lpr/common_source/fatal.c:fatal() still relies on lp{c,d,q,r,rm}
to set name = argv[0], when it should really use getprogname()
- lpr doesn't really need fatal2() as it is identical to errx()
All fatal2() calls should be replaced with errx() calls. Then
fatal2() can be removed. (Simillar to what was done in the
FreeBSD and OpenBSD versions.)
If others agree that these are "good" changes then I'll submit
a PR (unless someone wants to commit directly.)
Note the diffs below also include changes for PR bin/20890 and
standards/17916. Hint, hint... :-)
Index: common_source/common_vars.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/lpr/common_source/common_vars.c,v
retrieving revision 1.1
diff -u -r1.1 common_vars.c
--- common_source/common_vars.c 1999/12/05 22:10:57 1.1
+++ common_source/common_vars.c 2003/03/26 03:09:23
@@ -47,7 +47,6 @@
#include "pathnames.h"
-char *name; /* program name */
char *printer; /* printer name */
char host[MAXHOSTNAMELEN+1]; /* host machine name */
char *from = host; /* client's machine name */
Index: common_source/fatal.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/lpr/common_source/fatal.c,v
retrieving revision 1.3
diff -u -r1.3 fatal.c
--- common_source/fatal.c 2002/07/14 15:27:58 1.3
+++ common_source/fatal.c 2003/03/26 03:09:23
@@ -59,7 +59,7 @@
va_start(ap, msg);
if (from != host)
(void)printf("%s: ", host);
- (void)printf("%s: ", name);
+ (void)printf("%s: ", getprogname());
if (printer)
(void)printf("%s: ", printer);
(void)vprintf(msg, ap);
Index: common_source/lp.h
===================================================================
RCS file: /cvsroot/src/usr.sbin/lpr/common_source/lp.h,v
retrieving revision 1.16
diff -u -r1.16 lp.h
--- common_source/lp.h 2002/07/14 15:27:58 1.16
+++ common_source/lp.h 2003/03/26 03:09:23
@@ -83,7 +83,6 @@
extern char line[BUFSIZ];
extern char *bp; /* pointer into printcap buffer */
-extern char *name; /* program name */
extern char *printer; /* printer name */
/* host machine name */
extern char host[MAXHOSTNAMELEN + 1];
Index: lp/lp
===================================================================
RCS file: /cvsroot/src/usr.sbin/lpr/lp/lp,v
retrieving revision 1.4
diff -u -r1.4 lp
--- lp/lp 1998/07/07 17:05:28 1.4
+++ lp/lp 2003/03/26 03:09:23
@@ -42,6 +42,7 @@
ncopies=""
symlink="-s"
+reqid="-R"
# Posix says LPDEST gets precedence over PRINTER
dest=${LPDEST:-${PRINTER:-lp}}
@@ -57,7 +58,7 @@
c) # copy files before printing
symlink="";;
s) # silent
- : ;;
+ reqid="";;
d) # destination
dest="${OPTARG}";;
n) # number of copies
@@ -71,4 +72,4 @@
shift $(($OPTIND - 1))
-exec /usr/bin/lpr "-P${dest}" ${symlink} ${ncopies} "$@"
+exec /usr/bin/lpr "-P${dest}" ${reqid} ${symlink} ${ncopies} "$@"
Index: lpc/lpc.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/lpr/lpc/lpc.c,v
retrieving revision 1.14
diff -u -r1.14 lpc.c
--- lpc/lpc.c 2002/07/20 08:40:18 1.14
+++ lpc/lpc.c 2003/03/26 03:09:23
@@ -97,7 +97,6 @@
euid = geteuid();
uid = getuid();
seteuid(uid);
- name = argv[0];
openlog("lpd", 0, LOG_LPR);
if (--argc > 0) {
Index: lpd/lpd.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/lpr/lpd/lpd.c,v
retrieving revision 1.44
diff -u -r1.44 lpd.c
--- lpd/lpd.c 2002/10/26 01:46:31 1.44
+++ lpd/lpd.c 2003/03/26 03:09:24
@@ -161,7 +161,6 @@
uid = getuid();
gethostname(host, sizeof(host));
host[sizeof(host) - 1] = '\0';
- name = argv[0];
errs = 0;
while ((i = getopt(argc, argv, "b:dln:srw:W")) != -1)
Index: lpq/lpq.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/lpr/lpq/lpq.c,v
retrieving revision 1.12
diff -u -r1.12 lpq.c
--- lpq/lpq.c 2002/07/14 15:28:00 1.12
+++ lpq/lpq.c 2003/03/26 03:09:24
@@ -88,7 +88,6 @@
euid = geteuid();
uid = getuid();
seteuid(uid);
- name = *argv;
if (gethostname(host, sizeof(host)))
err(1, "lpq: gethostname");
host[sizeof(host) - 1] = '\0';
Index: lpr/lpr.1
===================================================================
RCS file: /cvsroot/src/usr.sbin/lpr/lpr/lpr.1,v
retrieving revision 1.12
diff -u -r1.12 lpr.1
--- lpr/lpr.1 2003/02/25 10:36:12 1.12
+++ lpr/lpr.1 2003/03/26 03:09:24
@@ -41,7 +41,7 @@
.Nd off line print
.Sh SYNOPSIS
.Nm
-.Op Fl cdfghlmnprstv
+.Op Fl cdfghlmnqprRstv
.Bk -words
.Op Fl P Ar printer
.Ek
@@ -132,6 +132,8 @@
Suppress the printing of the burst page.
.It Fl m
Send mail upon completion.
+.It Fl q
+Queue the print job but do not start the spooling daemon.
.It Fl r
Remove the file upon completion of spooling or upon completion of
printing (with the
@@ -146,6 +148,20 @@
to link data files rather than trying to copy them so large files can be
printed. This means the files should
not be modified or removed until they have been printed.
+.El
+.Pp
+Normally
+.Nm
+works silently except for diaganostic messages. The following
+option changes this behavior.
+.Bl -tag -width indent
+.It Fl R
+Writes a message to standard output containing the unique number which
+is used to identify this job. This number can be used to cancel (see
+.Xr lprm 1 )
+or find the status (see
+.Xr lpq 1 )
+of the job.
.El
.Pp
The remaining options apply to copies, the page display, and headers:
Index: lpr/lpr.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/lpr/lpr/lpr.c,v
retrieving revision 1.23
diff -u -r1.23 lpr.c
--- lpr/lpr.c 2002/07/14 15:28:00 1.23
+++ lpr/lpr.c 2003/03/26 03:09:24
@@ -94,7 +94,9 @@
static int ncopies = 1; /* # of copies to make */
static const char *person; /* user name */
static int qflag; /* q job, but don't exec daemon */
+static int reqid; /* request id */
static int rflag; /* remove files upon completion */
+static int Rflag; /* print request id - like POSIX lp */
static int sflag; /* symbolic link flag */
static int tfd; /* control file descriptor */
static char *tfname; /* tmp copy of cf before linking */
@@ -108,8 +110,6 @@
static void chkprinter(char *);
static void cleanup(int);
static void copy(int, char []);
-static void fatal2(const char *, ...)
- __attribute__((__format__(__printf__, 1, 2)));
static char *itoa(int);
static char *linked(char *);
static char *lmktemp(char *, int, int);
@@ -130,6 +130,7 @@
char buf[MAXPATHLEN];
int i, f, errs, c;
struct stat stb;
+ int nrequests = 0;
euid = geteuid();
uid = getuid();
@@ -144,14 +145,13 @@
if (signal(SIGTERM, SIG_IGN) != SIG_IGN)
signal(SIGTERM, cleanup);
- name = argv[0];
gethostname(host, sizeof (host));
host[sizeof(host) - 1] = '\0';
openlog("lpd", 0, LOG_LPR);
errs = 0;
while ((c = getopt(argc, argv,
- ":#:1:2:3:4:C:J:P:T:U:cdfghi:lmnprstvw:")) != -1) {
+ ":#:1:2:3:4:C:J:P:RT:U:cdfghi:lmnqprstvw:")) != -1) {
switch (c) {
case '#': /* n copies */
@@ -183,6 +183,10 @@
printer = optarg;
break;
+ case 'R': /* print request id */
+ Rflag++;
+ break;
+
case 'T': /* pr's title line */
title = optarg;
break;
@@ -258,9 +262,9 @@
printer = DEFLP;
chkprinter(printer);
if (SC && ncopies > 1)
- fatal2("multiple copies are not allowed");
+ errx(1, "multiple copies are not allowed");
if (MC > 0 && ncopies > MC)
- fatal2("only %ld copies are allowed", MC);
+ errx(1, "only %ld copies are allowed", MC);
/*
* Get the identity of the person doing the lpr using the same
* algorithm as lprm.
@@ -268,7 +272,7 @@
userid = getuid();
if (userid != DU || person == 0) {
if ((pw = getpwuid(userid)) == NULL)
- fatal2("Who are you?");
+ errx(1, "Who are you?");
person = pw->pw_name;
}
/*
@@ -276,7 +280,7 @@
*/
if (RG != NULL && userid != DU) {
if ((gptr = getgrnam(RG)) == NULL)
- fatal2("Restricted group specified incorrectly");
+ errx(1, "Restricted group specified incorrectly");
if (gptr->gr_gid != getgid()) {
while (*gptr->gr_mem != NULL) {
if ((strcmp(person, *gptr->gr_mem)) == 0)
@@ -284,7 +288,7 @@
gptr->gr_mem++;
}
if (*gptr->gr_mem == NULL)
- fatal2("Not a member of the restricted group");
+ errx(1, "Not a member of the restricted group");
}
}
/*
@@ -292,7 +296,7 @@
*/
(void)snprintf(buf, sizeof buf, "%s/%s", SD, LO);
if (userid && stat(buf, &stb) == 0 && (stb.st_mode & S_IXGRP))
- fatal2("Printer queue is disabled");
+ errx(1, "Printer queue is disabled");
/*
* Initialize the control file.
*/
@@ -329,17 +333,21 @@
/*
* Read the files and spool them.
*/
- if (argc == 0)
+ if (argc == 0) {
+ nrequests++;
copy(0, " ");
+ }
else while (argc--) {
+ nrequests++;
if (argv[0][0] == '-' && argv[0][1] == '\0') {
/* use stdin */
copy(0, " ");
argv++;
continue;
}
- if ((f = test(arg = *argv++)) < 0)
+ if ((f = test(arg = *argv++)) < 0) {
continue; /* file unreasonable */
+ }
if (sflag && (cp = linked(arg)) != NULL) {
(void)snprintf(buf, sizeof buf,
@@ -358,17 +366,18 @@
continue;
}
if (sflag)
- printf("%s: %s: not linked, copying instead\n", name, arg);
+ warnx("%s: not linked, copying instead", arg);
seteuid(uid);
if ((i = open(arg, O_RDONLY)) < 0) {
seteuid(uid);
- printf("%s: cannot open %s\n", name, arg);
+ warn("cannot open %s", arg);
continue;
} else {
copy(i, arg);
(void)close(i);
- if (f && unlink(arg) < 0)
- printf("%s: %s: not removed\n", name, arg);
+ if (f && unlink(arg) < 0) {
+ warn("%s: not removed", arg);
+ }
}
seteuid(uid);
}
@@ -386,24 +395,27 @@
if (read(tfd, &c, 1) == 1 &&
lseek(tfd, (off_t)0, 0) == 0 &&
write(tfd, &c, 1) != 1) {
- printf("%s: cannot touch %s\n", name, tfname);
+ warn("cannot touch %s", tfname);
tfname[inchar]++;
cleanup(0);
}
(void)close(tfd);
}
if (link(tfname, cfname) < 0) {
- printf("%s: cannot rename %s\n", name, cfname);
+ warn("cannot rename %s", cfname);
tfname[inchar]++;
cleanup(0);
}
unlink(tfname);
seteuid(uid);
+ if (Rflag)
+ printf("Request id is %d\n", reqid);
if (qflag) /* just q things up */
- exit(0);
+ exit(nact != nrequests);
if (!startdaemon(printer))
printf("jobs queued, but cannot start daemon.\n");
- exit(0);
+
+ exit(nact != nrequests);
}
cleanup(0);
#ifdef __GNUC__
@@ -431,7 +443,7 @@
nr = nc = 0;
while ((i = read(f, buf, sizeof buf)) > 0) {
if (write(fd, buf, i) != i) {
- printf("%s: %s: temp file write error\n", name, n);
+ warn("%s", n);
break;
}
nc += i;
@@ -439,15 +451,15 @@
nc -= sizeof buf;
nr++;
if (MX > 0 && nr > MX) {
- printf("%s: %s: copy file is too large "
- "(check :mx:?)\n", name, n);
+ warnx("%s: copy file is too large "
+ "(check :mx:?)", n);
break;
}
}
}
(void)close(fd);
if (nc==0 && nr==0)
- printf("%s: %s: empty input file\n", name, f ? n : "stdin");
+ warnx("%s: empty input file", f ? n : "stdin");
else
nact++;
}
@@ -527,17 +539,17 @@
f = open(n, O_WRONLY | O_EXCL | O_CREAT, FILMOD);
(void)umask(oldumask);
if (f < 0) {
- printf("%s: cannot create %s\n", name, n);
+ warn("cannot create %s", n);
cleanup(0);
}
if (fchown(f, userid, -1) < 0) {
- printf("%s: cannot chown %s\n", name, n);
+ warn("cannot chown %s", n);
cleanup(0); /* cleanup does exit */
}
seteuid(uid);
if (++n[inchar] > 'z') {
if (++n[inchar-2] == 't') {
- printf("too many files - break up the job\n");
+ warnx("too many files - break up the job");
cleanup(0);
}
n[inchar] = 'A';
@@ -592,29 +604,29 @@
seteuid(uid);
if (access(file, 4) < 0) {
- printf("%s: cannot access %s\n", name, file);
+ warn("cannot access %s", file);
goto bad;
}
if (stat(file, &statb) < 0) {
- printf("%s: cannot stat %s\n", name, file);
+ warn("cannot stat %s", file);
goto bad;
}
if (S_ISDIR(statb.st_mode)) {
- printf("%s: %s is a directory\n", name, file);
+ warnx("%s is a directory", file);
goto bad;
}
if (statb.st_size == 0) {
- printf("%s: %s is an empty file\n", name, file);
+ warnx("%s is an empty file", file);
goto bad;
}
if ((fd = open(file, O_RDONLY)) < 0) {
- printf("%s: cannot open %s\n", name, file);
+ warn("cannot open %s", file);
goto bad;
}
if (read(fd, &execb, sizeof(execb)) == sizeof(execb) &&
!N_BADMAG(execb)) {
- printf("%s: %s is an executable program and is unprintable",
- name, file);
+ warnx("%s is an executable program and is unprintable",
+ file);
(void)close(fd);
goto bad;
}
@@ -634,7 +646,7 @@
if (fd == 0)
return(1);
}
- printf("%s: %s: is not removable by you\n", name, file);
+ warnx("%s: is not removable by you", file);
}
return(0);
bad:
@@ -667,9 +679,9 @@
int status;
if ((status = cgetent(&bp, printcapdb, s)) == -2)
- fatal2("cannot open printer description file");
+ errx(1, "cannot open printer description file");
else if (status == -1)
- fatal2("%s: unknown printer", s);
+ errx(1, "%s: unknown printer", s);
if (cgetstr(bp, "sd", &SD) == -1)
SD = _PATH_DEFSPOOL;
if (cgetstr(bp, "lo", &LO) == -1)
@@ -697,12 +709,10 @@
(void)snprintf(buf, BUFSIZ, "%s/.seq", SD);
seteuid(euid);
if ((fd = open(buf, O_RDWR|O_CREAT, 0661)) < 0) {
- printf("%s: cannot create %s\n", name, buf);
- exit(1);
+ err(1, "cannot create %s", buf);
}
if (flock(fd, LOCK_EX)) {
- printf("%s: cannot lock %s\n", name, buf);
- exit(1);
+ err(1, "cannot lock %s", buf);
}
seteuid(uid);
n = 0;
@@ -713,6 +723,7 @@
n = n * 10 + (*cp++ - '0');
}
}
+ reqid = n;
len = strlen(SD) + strlen(host) + 8;
tfname = lmktemp("tf", n, len);
cfname = lmktemp("cf", n, len);
@@ -734,26 +745,11 @@
char *s;
if ((s = malloc(len)) == NULL)
- fatal2("out of memory");
+ errx(1, "out of memory");
(void)snprintf(s, len, "%s/%sA%03d%s", SD, id, num, host);
return(s);
}
-#include <stdarg.h>
-
-static void
-fatal2(const char *msg, ...)
-{
- va_list ap;
-
- va_start(ap, msg);
- printf("%s: ", name);
- vprintf(msg, ap);
- putchar('\n');
- va_end(ap);
- exit(1);
-}
-
static void
usage(void)
{
@@ -761,6 +757,6 @@
fprintf(stderr, "%s\n%s\n",
"usage: lpr [-Pprinter] [-#num] [-C class] [-J job] [-T title] "
"[-U user]",
- "[-i[numcols]] [-1234 font] [-wnum] [-cdfghlmnprstv] [name ...]");
+ "[-i[numcols]] [-1234 font] [-wnum] [-cdfghlmnqprstv] [name ...]");
exit(1);
}
Index: lprm/lprm.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/lpr/lprm/lprm.c,v
retrieving revision 1.13
diff -u -r1.13 lprm.c
--- lprm/lprm.c 2002/07/14 15:28:01 1.13
+++ lprm/lprm.c 2003/03/26 03:09:24
@@ -95,7 +95,6 @@
uid = getuid();
euid = geteuid();
seteuid(uid); /* be safe */
- name = argv[0];
gethostname(host, sizeof(host));
host[sizeof(host) - 1] = '\0';
openlog("lpd", 0, LOG_LPR);