Fix some bugs and general brain damage in mounttab:

- Declare mtabhead as an extern in mounttab.h and define it only in
  mounttab.c.
- Remove shared global `verbose' and instead pass it as a parameter.
- Remove the `mtabp' argument to read_mtab(). It served no purpose
  whatsoever, although read_mtab() did use it as a temporary local
  variable.
- Don't check for impossible conditions when parsing mounttab, and
  do detect zero-length fields.
- Correctly test for strtoul() failures - just testing ERANGE is wrong.
- Include a field name in syslog errors, and avoid passing NULL to
  a syslog %s field.
- Don't test if arrays are NULL.
- If there are duplicates when writing out mounttab, keep the last
  entry instead of the first, as it will have a later timestamp.
- Fix a few formatting issues.

Update rpc.umntall and umount to match the mounttab interface changes.

3 files changed, 66 insertions, 68 deletions

diff --git a/usr.sbin/rpc.umntall/mounttab.c b/usr.sbin/rpc.umntall/mounttab.c
index 551a03d..d4b1865 100644
--- a/usr.sbin/rpc.umntall/mounttab.c
+++ b/usr.sbin/rpc.umntall/mounttab.c
@@ -37,6 +37,7 @@ static const char rcsid[] =
 
 #include <err.h>
 #include <errno.h>
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -44,9 +45,10 @@ static const char rcsid[] =
 
 #include "mounttab.h"
 
-int verbose;
 struct mtablist *mtabhead;
 
+static void badline(char *field, char *bad);
+
 /*
  * Add an entry to PATH_MOUNTTAB for each mounted NFS filesystem,
  * so the client can notify the NFS server even after reboot.
@@ -54,14 +56,12 @@ struct mtablist *mtabhead;
 int
 add_mtab(char *hostp, char *dirp) {
 	FILE *mtabfile;
-	time_t *now;
 
-	now = NULL;
 	if ((mtabfile = fopen(PATH_MOUNTTAB, "a")) == NULL)
 		return (0);
 	else {
 		fprintf(mtabfile, "%ld\t%s\t%s\n",
-		    (long)time(now), hostp, dirp);
+		    (long)time(NULL), hostp, dirp);
 		fclose(mtabfile);
 		return (1);
 	}
@@ -71,12 +71,13 @@ add_mtab(char *hostp, char *dirp) {
  * Read mounttab line for line and return struct mtablist.
  */
 int
-read_mtab(struct mtablist *mtabp) {
-	struct mtablist **mtabpp;
+read_mtab() {
+	struct mtablist **mtabpp, *mtabp;
 	char *hostp, *dirp, *cp;
 	char str[STRSIZ];
-	char *timep;
+	char *timep, *endp;
 	time_t time;
+	u_long ultmp;
 	FILE *mtabfile;
 
 	if ((mtabfile = fopen(PATH_MOUNTTAB, "r")) == NULL) {
@@ -95,25 +96,26 @@ read_mtab(struct mtablist *mtabp) {
 		if (*cp == '#' || *cp == ' ' || *cp == '\n')
 			continue;
 		timep = strsep(&cp, " \t\n");
-		if (timep == NULL || *timep == ' ' || *timep == '\n') {
-			badline(timep);
+		if (timep == NULL || *timep == '\0') {
+			badline("time", timep);
 			continue;
 		}
 		hostp = strsep(&cp, " \t\n");
-		if (hostp == NULL || *hostp == ' ' || *hostp == '\n') {
-			badline(hostp);
+		if (hostp == NULL || *hostp == '\0') {
+			badline("host", hostp);
 			continue;
 		}
 		dirp = strsep(&cp, " \t\n");
-		if (dirp == NULL || *dirp == ' ' || *dirp == '\n') {
-			badline(dirp);
+		if (dirp == NULL || *dirp == '\0') {
+			badline("dir", dirp);
 			continue;
 		}
-		time = strtoul(timep, (char **)NULL, 10);
-		if (errno == ERANGE) {
-			badline(timep);
+		ultmp = strtoul(timep, &endp, 10);
+		if (ultmp == ULONG_MAX || *endp != '\0') {
+			badline("time", timep);
 			continue;
 		}
+		time = ultmp;
 		if ((mtabp = malloc(sizeof (struct mtablist))) == NULL) {
 			syslog(LOG_ERR, "malloc");
 			fclose(mtabfile);
@@ -137,8 +139,8 @@ read_mtab(struct mtablist *mtabp) {
  * Unlink PATH_MOUNTAB if no entry is left.
  */
 int
-write_mtab() {
-	struct mtablist *mtabp;
+write_mtab(int verbose) {
+	struct mtablist *mtabp, *mp;
 	FILE *mtabfile;
 	int line;
 
@@ -148,18 +150,23 @@ write_mtab() {
 	}
 	line = 0;
 	for (mtabp = mtabhead; mtabp != NULL; mtabp = mtabp->mtab_next) {
-		if (mtabp->mtab_host != NULL &&
-		    strlen(mtabp->mtab_host) > 0) {
-			fprintf(mtabfile, "%ld\t%s\t%s\n",
-			    (long)mtabp->mtab_time, mtabp->mtab_host,
-			    mtabp->mtab_dirp);
-			if (verbose) {
-				warnx("write entry " "%s:%s",
-				    mtabp->mtab_host, mtabp->mtab_dirp);
-			}
-			clean_mtab(mtabp->mtab_host, mtabp->mtab_dirp);
-			line++;
-		}		
+		if (mtabp->mtab_host[0] == '\0')
+			continue;
+		/* Skip if a later (hence more recent) entry is identical. */
+		for (mp = mtabp->mtab_next; mp != NULL; mp = mp->mtab_next)
+			if (strcmp(mtabp->mtab_host, mp->mtab_host) == 0 &&
+			    strcmp(mtabp->mtab_dirp, mp->mtab_dirp) == 0)
+				break;
+		if (mp != NULL)
+			continue;
+
+		fprintf(mtabfile, "%ld\t%s\t%s\n",
+		    (long)mtabp->mtab_time, mtabp->mtab_host,
+		    mtabp->mtab_dirp);
+		if (verbose)
+			warnx("write mounttab entry %s:%s",
+			    mtabp->mtab_host, mtabp->mtab_dirp);
+		line++;
 	}
 	fclose(mtabfile);
 	if (line == 0) {
@@ -175,30 +182,23 @@ write_mtab() {
  * Mark the entries as clean where RPC calls have been done successfully.
  */
 void
-clean_mtab(char *hostp, char *dirp) {
+clean_mtab(char *hostp, char *dirp, int verbose) {
 	struct mtablist *mtabp;
 	char *host;
 
+	/* Copy hostp in case it points to an entry that we are zeroing out. */
 	host = strdup(hostp);
 	for (mtabp = mtabhead; mtabp != NULL; mtabp = mtabp->mtab_next) {
-		if (mtabp->mtab_host != NULL &&
-		    strcmp(mtabp->mtab_host, host) == 0) {
-			if (dirp == NULL) {
-				if (verbose) {
-					warnx("delete entries "
-					    "host %s", host);
-				}
-				bzero(mtabp->mtab_host, RPCMNT_NAMELEN);
-			} else {
-				if (strcmp(mtabp->mtab_dirp, dirp) == 0) {
-					if (verbose) {
-						warnx("delete entry "
-						    "%s:%s", host, dirp);
-					}
-					bzero(mtabp->mtab_host, RPCMNT_NAMELEN);
-				}
-			}
-		}
+		if (strcmp(mtabp->mtab_host, hostp) != 0)
+			continue;
+		if (dirp != NULL && strcmp(mtabp->mtab_dirp, dirp) != 0)
+			continue;
+
+		if (verbose)
+			warnx("delete mounttab entry%s %s:%s",
+			    (dirp == NULL) ? " by host" : "",
+			    mtabp->mtab_host, mtabp->mtab_dirp);
+		bzero(mtabp->mtab_host, RPCMNT_NAMELEN);
 	}
 	free(host);
 }
@@ -209,20 +209,18 @@ clean_mtab(char *hostp, char *dirp) {
 void
 free_mtab() {
 	struct mtablist *mtabp;
-	struct mtablist *mtab_next;
 
-	for (mtabp = mtabhead; mtabp != NULL; mtabp = mtab_next) {
-		mtab_next = mtabp->mtab_next;
+	while ((mtabp = mtabhead) != NULL) {
+		mtabhead = mtabhead->mtab_next;
 		free(mtabp);
-		mtabp = mtab_next;
 	}
 }
 
 /*
  * Print bad lines to syslog.
  */
-void
-badline(char *bad) {
-
-	syslog(LOG_ERR, "skip bad line in mounttab with entry %s", bad);
+static void
+badline(char *field, char *bad) {
+	syslog(LOG_ERR, "bad mounttab %s field '%s'", field,
+	    (bad == NULL) ? "<null>" : bad);
 }
diff --git a/usr.sbin/rpc.umntall/mounttab.h b/usr.sbin/rpc.umntall/mounttab.h
index 21d789a..acf6c79 100644
--- a/usr.sbin/rpc.umntall/mounttab.h
+++ b/usr.sbin/rpc.umntall/mounttab.h
@@ -37,9 +37,10 @@ struct mtablist {
 	struct mtablist *mtab_next;
 };
 
+extern struct mtablist *mtabhead;
+
 int	add_mtab(char *, char *);
-void	badline (char *);
-void	clean_mtab (char *, char *);
-int	read_mtab (struct mtablist *);
-int	write_mtab (void);
-void	free_mtab (void);
+void	clean_mtab(char *, char *, int);
+int	read_mtab(void);
+int	write_mtab(int);
+void	free_mtab(void);
diff --git a/usr.sbin/rpc.umntall/rpc.umntall.c b/usr.sbin/rpc.umntall/rpc.umntall.c
index cf570f5..f4395a8 100644
--- a/usr.sbin/rpc.umntall/rpc.umntall.c
+++ b/usr.sbin/rpc.umntall/rpc.umntall.c
@@ -54,8 +54,6 @@ static int is_mounted (char *, char *);
 static void usage (void);
 int	xdr_dir (XDR *, char *);
 
-struct mtablist *mtabhead;
-
 int
 main(int argc, char **argv) {
 	int ch, keep, success, pathlen;
@@ -100,7 +98,7 @@ main(int argc, char **argv) {
 	time(&now);
 
 	/* Read PATH_MOUNTTAB. */
-	if (!read_mtab(NULL)) {
+	if (!read_mtab()) {
 		if (verbose)
 			warnx("no mounttab entries (%s does not exist)",
 			    PATH_MOUNTTAB);
@@ -133,7 +131,8 @@ main(int argc, char **argv) {
 					warnx("umount RPC for %s:%s succeeded",
 					    mtab->mtab_host, mtab->mtab_dirp);
 				/* Remove all entries for this host + path. */
-				clean_mtab(mtab->mtab_host, mtab->mtab_dirp);
+				clean_mtab(mtab->mtab_host, mtab->mtab_dirp,
+				    verbose);
 			}
 		}
 		success = 1;
@@ -158,11 +157,11 @@ main(int argc, char **argv) {
 		}
 		/* If successful, remove any corresponding mounttab entries. */
 		if (success)
-			clean_mtab(host, path);
+			clean_mtab(host, path, verbose);
 	}
 	/* Write and unlink PATH_MOUNTTAB if necessary */
 	if (success)
-		success = write_mtab();
+		success = write_mtab(verbose);
 	free_mtab();
 	exit (success ? 0 : 1);
 }