Rewrite of much of lpc/cmds.c ...

So, as I compare freebsd's lpc to RPI's lpc, I notice two things
about many of lpc's commands:

    1) a lot of commands duplicate the code to modify the
       state of a queue.
    2) many of them have 'euid' (the priv uid) in effect
       when they write out any error messages.  this seems
       like a bad idea.

That sounds like a tiny thread, but as I pull on it I ended
up rewriting more and more of lpc/cmds.c.  I figure I better
stop at this point, before I rewrite the entire thing.  Here
is an update which basically rewrites

      abort    disable  enable  restart
      start    stop     up

It is also available at:
http://people.freebsd.org/~gad/lpr/lpc-cmds.diff

This just adds the new implementations. The previous versions
remain available as "x"-ed commands (eg:  xabort, xdisable, etc).
This way, if you think there is something odd about the new
implementation, you can resort to the previous implementation
and see if it's really any different.  A later update will
remove the "x"-ed versions.

 From a user standpoint, this is meant to be an almost invisible
change, except that some error situations give a better error
message (IMO).  One other visible change is that the 'restart'
command will only print the printer's name once, instead of
printing it once for the 'abort' part, and a second time for
the 'restart' part.  That had always annoyed me...

I would like to commit this to current around June 15th.  The
two main workhorse routines are set_qstate and kill_qtask.
Since this does not delete anything, you'd have to look at
the original lpc source to see what the old code did.

Here is the update:

Index: common_source/common.c
===================================================================
RCS file: /home/ncvs/src/usr.sbin/lpr/common_source/common.c,v
retrieving revision 1.25
diff -u -r1.25 common.c
--- common_source/common.c	28 May 2002 19:21:10 -0000	1.25
+++ common_source/common.c	8 Jun 2002 21:16:56 -0000
@@ -50,6 +50,7 @@
  #include <sys/types.h>

  #include <dirent.h>
+#include <errno.h>
  #include <fcntl.h>
  #include <stdio.h>
  #include <stdlib.h>
@@ -250,6 +251,134 @@
  		snprintf(buf, len, "%s/%s", pp->spool_dir, pp->status_file);

  	return buf;
+}
+
+/*
+ * Routine to change operational state of a print queue.  The operational
+ * state is indicated by the access bits on the lock file for the queue.
+ * At present, this is only called from various routines in lpc/cmds.c.
+ *
+ *   XXX - Note that this works by changing access-bits on the
+ *         file, and you can only do that if you are the owner
+ *         of the file, or root.  Thus, this won't really work
+ *	   for userids in the "LPR_OPER" group, unless lpc is
+ *	   running setuid to root or maybe setuid to daemon.
+ *	   By default, lpc is installed setgid to daemon, but
+ *	   does not run setuid.
+ */
+int
+set_qstate(int action, const char *lfname)
+{
+	struct stat stbuf;
+	mode_t chgbits, newbits, oldmask;
+	const char *failmsg, *okmsg;
+	int chres, errsav, fd, res, statres;
+
+	/*
+	 * Find what the current access-bits are.
+	 */
+	memset(&stbuf, 0, sizeof(stbuf));
+	seteuid(euid);
+	statres = stat(lfname, &stbuf);
+	errsav = errno;
+	seteuid(uid);
+	if ((statres < 0) && (errsav != ENOENT)) {
+		printf("\tcannot stat() lock file\n");
+		return (SQS_STATFAIL);
+		/* NOTREACHED */
+	}
+
+	/*
+	 * Determine which bit(s) should change for the requested action.
+	 */
+	chgbits = stbuf.st_mode;
+	newbits = LOCK_FILE_MODE;
+	okmsg = NULL;
+	failmsg = NULL;
+	if (action & SQS_DISABLEQ) {
+		chgbits |= LFM_QUEUE_DIS;
+		newbits |= LFM_QUEUE_DIS;
+		okmsg = "queuing disabled";
+		failmsg = "disable queuing";
+	}
+	if (action & SQS_STOPP) {
+		chgbits |= LFM_PRINT_DIS;
+		newbits |= LFM_PRINT_DIS;
+		okmsg = "printing disabled";
+		failmsg = "disable printing";
+	}
+	if (action & SQS_ENABLEQ) {
+		chgbits &= ~LFM_QUEUE_DIS;
+		newbits &= ~LFM_QUEUE_DIS;
+		okmsg = "queuing enabled";
+		failmsg = "enable queuing";
+	}
+	if (action & SQS_STARTP) {
+		chgbits &= ~LFM_PRINT_DIS;
+		newbits &= ~LFM_PRINT_DIS;
+		okmsg = "printing enabled";
+		failmsg = "enable printing";
+	}
+	if (okmsg == NULL) {
+		/* This routine was called with an invalid action. */
+		printf("\t<error in set_qstate!>\n");
+		return (SQS_PARMERR);
+		/* NOTREACHED */
+	}
+
+	res = 0;
+	if (statres >= 0) {
+		/* The file already exists, so change the access. */
+		seteuid(euid);
+		chres = chmod(lfname, chgbits);
+		errsav = errno;
+		seteuid(uid);
+		res = SQS_CHGOK;
+		if (res < 0)
+			res = SQS_CHGFAIL;
+	} else if (newbits == LOCK_FILE_MODE) {
+		/*
+		 * The file does not exist, but the state requested is
+		 * the same as the default state when no file exists.
+		 * Thus, there is no need to create the file.
+		 */
+		res = SQS_SKIPCREOK;
+	} else {
+		/*
+		 * The file did not exist, so create it with the
+		 * appropriate access bits for the requested action.
+		 * Push a new umask around that create, to make sure
+		 * all the read/write bits are set as desired.
+		 */
+		oldmask = umask(S_IWOTH);
+		seteuid(euid);
+		fd = open(lfname, O_WRONLY|O_CREAT, newbits);
+		errsav = errno;
+		seteuid(uid);
+		umask(oldmask);
+		res = SQS_CREFAIL;
+		if (fd >= 0) {
+			res = SQS_CREOK;
+			close(fd);
+		}
+	}
+
+	switch (res) {
+	case SQS_CHGOK:
+	case SQS_CREOK:
+	case SQS_SKIPCREOK:
+		printf("\t%s\n", okmsg);
+		break;
+	case SQS_CREFAIL:
+		printf("\tcannot create lock file: %s\n",
+		    strerror(errsav));
+		break;
+	default:
+		printf("\tcannot %s: %s\n", failmsg, strerror(errsav));
+		break;
+	}
+
+	return (res);
  }

  /* routine to get a current timestamp, optionally in a standard-fmt string */
Index: common_source/lp.h
===================================================================
RCS file: /home/ncvs/src/usr.sbin/lpr/common_source/lp.h,v
retrieving revision 1.20
diff -u -r1.20 lp.h
--- common_source/lp.h	23 Apr 2002 00:06:10 -0000	1.20
+++ common_source/lp.h	8 Jun 2002 21:16:56 -0000
@@ -223,6 +223,23 @@
  #define	TEMP_FILE_MODE	(S_IRUSR | S_IWUSR | S_IRGRP | 
S_IWGRP | S_IROTH)

  /*
+ * Bit-flags for set_qstate() actions, followed by the return values.
+ */
+#define SQS_DISABLEQ	0x01	/* Disable the queuing of new jobs */
+#define SQS_STOPP	0x02	/* Stop the printing of jobs */
+#define SQS_ENABLEQ	0x10	/* Enable the queuing of new jobs */
+#define SQS_STARTP	0x20	/* Start the printing of jobs */
+
+#define SQS_PARMERR	-9	/* Invalid parameters from caller */
+#define SQS_CREFAIL	-3	/* File did not exist, and create failed */
+#define SQS_CHGFAIL	-2	/* File exists, but unable to change state */
+#define SQS_STATFAIL	-1	/* Unable to stat() the lock file */
+#define SQS_CHGOK	1	/* File existed, and the state was changed */
+#define SQS_CREOK	2	/* File did not exist, but was created OK */
+#define SQS_SKIPCREOK	3	/* File did not exist, and there was */
+				/* no need to create it */
+
+/*
   * Command codes used in the protocol.
   */
  #define	CMD_CHECK_QUE	'\1'
@@ -272,6 +289,7 @@
  void	 rmjob(const char *_printer);
  void	 rmremote(const struct printer *_pp);
  void	 setprintcap(char *_newfile);
+int	 set_qstate(int _action, const char *_lfname);
  void	 show(const char *_nfile, const char *_datafile, int _copies);
  int	 startdaemon(const struct printer *_pp);
  char	*status_file_name(const struct printer *_pp, char *_buf,
Index: lpc/cmds.c
===================================================================
RCS file: /home/ncvs/src/usr.sbin/lpr/lpc/cmds.c,v
retrieving revision 1.24
diff -u -r1.24 cmds.c
--- lpc/cmds.c	22 Apr 2002 23:28:42 -0000	1.24
+++ lpc/cmds.c	8 Jun 2002 21:16:56 -0000
@@ -70,9 +70,18 @@
  #include "extern.h"
  #include "pathnames.h"

+/*
+ * Return values from kill_qtask().
+ */
+#define KQT_LFERROR	-2
+#define KQT_KILLFAIL	-1
+#define KQT_NODAEMON	0
+#define KQT_KILLOK	1
+
  static void	 abortpr(struct printer *_pp, int _dis);
  static int	 doarg(char *_job);
  static int	 doselect(struct dirent *_d);
+static int	 kill_qtask(const char *lf);
  static void	 putmsg(struct printer *_pp, int _argc, char **_argv);
  static int	 sortq(const void *_a, const void *_b);
  static void	 startpr(struct printer *_pp, int _chgenable);
@@ -278,6 +287,140 @@
  }

  /*
+ * kill an existing daemon and disable printing.
+ */
+void
+abort_q(struct printer *pp)
+{
+	int killres, setres;
+	char lf[MAXPATHLEN];
+
+	lock_file_name(pp, lf, sizeof lf);
+	printf("%s:\n", pp->printer);
+
+	/*
+	 * Turn on the owner execute bit of the lock file to disable printing.
+	 */
+	setres = set_qstate(SQS_STOPP, lf);
+
+	/*
+	 * If set_qstate found that there already was a lock file, then
+	 * call a routine which will read that lock file and kill the
+	 * lpd-process which is listed in that lock file.  If the lock
+	 * file did not exist, then either there is no daemon running
+	 * for this queue, or there is one running but *it* could not
+	 * write a lock file (which means we can not determine the
+	 * process id of that lpd-process).
+	 */
+	switch (setres) {
+	case SQS_CHGOK:
+	case SQS_CHGFAIL:
+		/* Kill the process */
+		killres = kill_qtask(lf);
+		break;
+	case SQS_CREOK:
+	case SQS_CREFAIL:
+		printf("\tno daemon to abort\n");
+		break;
+	case SQS_STATFAIL:
+		printf("\tassuming no daemon to abort\n");
+		break;
+	default:
+		printf("\t<unexpected result (%d) from set_qstate>\n",
+		    setres);
+		break;
+	}
+
+}
+
+/*
+ * Kill the current daemon, to stop printing of the active job.
+ */
+static int
+kill_qtask(const char *lf)
+{
+	FILE *fp;
+	pid_t pid;
+	int errsav, killres, lockres, res;
+
+	seteuid(euid);
+	fp = fopen(lf, "r");
+	errsav = errno;
+	seteuid(uid);
+	res = KQT_NODAEMON;
+	if (fp == NULL) {
+		/*
+		 * If there is no lock file, then there is no daemon to
+		 * kill.  Any other error return means there is some
+		 * kind of problem with the lock file.
+		 */
+		if (errsav != ENOENT)
+			res = KQT_LFERROR;
+		goto killdone;
+	}
+
+	/* If the lock file is empty, then there is no daemon to kill */
+	if (getline(fp) == 0)
+		goto killdone;
+
+	/*
+	 * If the file can be locked without blocking, then there
+	 * no daemon to kill, or we should not try to kill it.
+	 *
+	 * XXX - not sure I understand the reasoning behind this...
+	 */
+	lockres = flock(fileno(fp), LOCK_SH|LOCK_NB);
+	(void) fclose(fp);
+	if (lockres == 0)
+		goto killdone;
+
+	pid = atoi(line);
+	if (pid < 0) {
+		/*
+		 * If we got a negative pid, then the contents of the
+		 * lock file is not valid.
+		 */
+		res = KQT_LFERROR;
+		goto killdone;
+	}
+
+	seteuid(uid);
+	killres = kill(pid, SIGTERM);
+	errsav = errno;
+	seteuid(uid);
+	if (killres == 0) {
+		res = KQT_KILLOK;
+		printf("\tdaemon (pid %d) killed\n", pid);
+	} else if (errno == ESRCH) {
+		res = KQT_NODAEMON;
+	} else {
+		res = KQT_KILLFAIL;
+		printf("\tWarning: daemon (pid %d) not killed:\n", pid);
+		printf("\t    %s\n", strerror(errsav));
+	}
+
+killdone:
+	switch (res) {
+	case KQT_LFERROR:
+		printf("\tcannot open lock file: %s\n",
+		    strerror(errsav));
+		break;
+	case KQT_NODAEMON:
+		printf("\tno daemon to abort\n");
+		break;
+	case KQT_KILLFAIL:
+	case KQT_KILLOK:
+		/* These two already printed messages to the user. */
+		break;
+	default:
+		printf("\t<internal error in kill_qtask>\n");
+		break;
+	}
+
+	return (res);
+}
+
+/*
   * Write a message into the status file.
   */
  static void
@@ -750,6 +893,21 @@
  }

  /*
+ * Enable queuing to the printer (allow lpr to add new jobs to the queue).
+ */
+void
+enable_q(struct printer *pp)
+{
+	int setres;
+	char lf[MAXPATHLEN];
+
+	lock_file_name(pp, lf, sizeof lf);
+	printf("%s:\n", pp->printer);
+
+	setres = set_qstate(SQS_ENABLEQ, lf);
+}
+
+/*
   * Disable queuing.
   */
  void
@@ -786,6 +944,21 @@
  }

  /*
+ * Disable queuing.
+ */
+void
+disable_q(struct printer *pp)
+{
+	int setres;
+	char lf[MAXPATHLEN];
+
+	lock_file_name(pp, lf, sizeof lf);
+	printf("%s:\n", pp->printer);
+
+	setres = set_qstate(SQS_DISABLEQ, lf);
+}
+
+/*
   * Disable queuing and printing and put a message into the status file
   * (reason for being down).
   */
@@ -924,6 +1097,37 @@
  }

  /*
+ * Kill and restart the daemon.
+ */
+void
+restart_q(struct printer *pp)
+{
+	int killres, setres, startok;
+	char lf[MAXPATHLEN];
+
+	lock_file_name(pp, lf, sizeof lf);
+	printf("%s:\n", pp->printer);
+
+	killres = kill_qtask(lf);
+
+	/*
+	 * XXX - if the kill worked, we should probably sleep for
+	 *      a second or so before trying to restart the queue.
+	 */
+
+	/* make sure the queue is set to print jobs */
+	setres = set_qstate(SQS_STARTP, lf);
+
+	seteuid(euid);
+	startok = startdaemon(pp);
+	seteuid(uid);
+	if (!startok)
+		printf("\tcouldn't restart daemon\n");
+	else
+		printf("\tdaemon restarted\n");
+}
+
+/*
   * Enable printing on the specified printer and startup the daemon.
   */
  void
@@ -962,6 +1166,30 @@
  }

  /*
+ * Enable printing on the specified printer and startup the daemon.
+ */
+void
+start_q(struct printer *pp)
+{
+	int setres, startok;
+	char lf[MAXPATHLEN];
+
+	lock_file_name(pp, lf, sizeof lf);
+	printf("%s:\n", pp->printer);
+
+	setres = set_qstate(SQS_STARTP, lf);
+
+	seteuid(euid);
+	startok = startdaemon(pp);
+	seteuid(uid);
+	if (!startok)
+		printf("\tcouldn't start daemon\n");
+	else
+		printf("\tdaemon started\n");
+	seteuid(uid);
+}
+
+/*
   * Print the status of the printer queue.
   */
  void
@@ -1064,6 +1292,28 @@
  	seteuid(uid);
  }

+/*
+ * Stop the specified daemon after completing the current job and disable
+ * printing.
+ */
+void
+stop_q(struct printer *pp)
+{
+	int setres;
+	char lf[MAXPATHLEN];
+
+	lock_file_name(pp, lf, sizeof lf);
+	printf("%s:\n", pp->printer);
+
+	setres = set_qstate(SQS_STOPP, lf);
+
+	if (setres >= 0) {
+		seteuid(euid);
+		upstat(pp, "printing disabled\n");
+		seteuid(uid);
+	}
+}
+
  struct	jobqueue **queue;
  int	nitems;
  time_t	mtime;
@@ -1236,4 +1486,27 @@
  up(struct printer *pp)
  {
  	startpr(pp, 2);
+}
+
+/*
+ * Enable both queuing & printing, and start printer (undo `down').
+ */
+void
+up_q(struct printer *pp)
+{
+	int setres, startok;
+	char lf[MAXPATHLEN];
+
+	lock_file_name(pp, lf, sizeof lf);
+	printf("%s:\n", pp->printer);
+
+	setres = set_qstate(SQS_ENABLEQ+SQS_STARTP, lf);
+
+	seteuid(euid);
+	startok = startdaemon(pp);
+	seteuid(uid);
+	if (!startok)
+		printf("\tcouldn't start daemon\n");
+	else
+		printf("\tdaemon started\n");
  }
Index: lpc/cmdtab.c
===================================================================
RCS file: /home/ncvs/src/usr.sbin/lpr/lpc/cmdtab.c,v
retrieving revision 1.4
diff -u -r1.4 cmdtab.c
--- lpc/cmdtab.c	25 Jun 2001 02:05:03 -0000	1.4
+++ lpc/cmdtab.c	8 Jun 2002 21:16:56 -0000
@@ -65,21 +65,28 @@
  #define PR	1	/* a privileged command */

  struct cmd cmdtab[] = {
-	{ "abort",	aborthelp,	PR,	0,		doabort },
+	{ "abort",	aborthelp,	PR,	0,		abort_q },
  	{ "clean",	cleanhelp,	PR,	init_clean,	clean_q },
-	{ "enable",	enablehelp,	PR,	0,		enable },
+	{ "enable",	enablehelp,	PR,	0,		enable_q },
  	{ "exit",	quithelp,	0,	quit,		0 },
-	{ "disable",	disablehelp,	PR,	0, 		disable },
+	{ "disable",	disablehelp,	PR,	0, 		disable_q },
  	{ "down",	downhelp,	PR,	down,		0 },
  	{ "help",	helphelp,	0,	help,		0 },
  	{ "quit",	quithelp,	0,	quit,		0 },
-	{ "restart",	restarthelp,	0,	0,		restart },
-	{ "start",	starthelp,	PR,	0,		startcmd },
+	{ "restart",	restarthelp,	0,	0,		restart_q },
+	{ "start",	starthelp,	PR,	0,		start_q },
  	{ "status",	statushelp,	0,	0,		status },
-	{ "stop",	stophelp,	PR,	0,		stop },
+	{ "stop",	stophelp,	PR,	0,		stop_q },
  	{ "tclean",	tcleanhelp,	0,	init_tclean,	clean_q },
  	{ "topq",	topqhelp,	PR,	topq,		0 },
-	{ "up",		uphelp,		PR,	0,		up },
+	{ "up",		uphelp,		PR,	0,		up_q },
+	{ "xabort",	aborthelp,	PR,	0,		doabort },
+	{ "xenable",	enablehelp,	PR,	0,		enable },
+	{ "xdisable",	disablehelp,	PR,	0, 		disable },
+	{ "xrestart",	restarthelp,	0,	0,		restart },
+	{ "xstart",	starthelp,	PR,	0,		startcmd },
+	{ "xstop",	stophelp,	PR,	0,		stop },
+	{ "xup",	uphelp,		PR,	0,		up },
  	{ "?",		helphelp,	0,	help,		0 },
  	{ 0, 0, 0, 0, 0},
  };
Index: lpc/extern.h
===================================================================
RCS file: /home/ncvs/src/usr.sbin/lpr/lpc/extern.h,v
retrieving revision 1.5
diff -u -r1.5 extern.h
--- lpc/extern.h	25 Jun 2001 02:05:03 -0000	1.5
+++ lpc/extern.h	8 Jun 2002 21:16:56 -0000
@@ -42,11 +42,14 @@


  __BEGIN_DECLS
+void	 abort_q(struct printer *_pp);
  void	 clean_q(struct printer *_pp);
  void	 disable(struct printer *_pp);
+void	 disable_q(struct printer *_pp);
  void	 doabort(struct printer *_pp);
  void	 down(int _argc, char *_argv[]);
  void	 enable(struct printer *_pp);
+void	 enable_q(struct printer *_pp);
  void	 generic(void (*_specificrtn)(struct printer *_pp),
  	    void (*_initcmd)(int _argc, char *_argv[]),
  	    int _argc, char *_argv[]);
@@ -55,11 +58,15 @@
  void	 init_tclean(int _argc, char *_argv[]);
  void	 quit(int _argc, char *_argv[]);
  void	 restart(struct printer *_pp);
+void	 restart_q(struct printer *_pp);
  void	 startcmd(struct printer *_pp);
+void	 start_q(struct printer *_pp);
  void	 status(struct printer *_pp);
  void	 stop(struct printer *_pp);
+void	 stop_q(struct printer *_pp);
  void	 topq(int _argc, char *_argv[]);
  void	 up(struct printer *_pp);
+void	 up_q(struct printer *_pp);
  __END_DECLS

  extern int NCMDS;

-- 
Garance Alistair Drosehn            =   [email protected]
Senior Systems Programmer           or  [email protected]
Rensselaer Polytechnic Institute    or  [email protected]

To Unsubscribe: send mail to [email protected]
with "unsubscribe freebsd-audit" in the body of the message