Fix file descriptor and memory leaks in pr(1)

Also, hook NetBSD's pr test into the build, and add three more test cases.

Reported by:	Coverity, Valgrind
CID:		271650 271651 271652 271653 271654 271655 271656 271656
CID:		271657 271658 271659 1006939 1006940 1006941 1006942 1009098
Reviewed by:	ngie
MFC after:	3 weeks
Sponsored by:	Spectra Logic Corp
Differential Revision:	https://reviews.freebsd.org/D9137

etc/mtree/BSD.tests.dist

@@ -642,6 +642,8 @@
         ..
         opensm
         ..
+        pr
+        ..
         printf
         ..
         sdiff

usr.bin/pr/Makefile

@@ -1,7 +1,13 @@
 #	@(#)Makefile	8.1 (Berkeley) 6/6/93
 # $FreeBSD$
 
+.include <src.opts.mk>
+
 PROG=	pr
 SRCS=	pr.c egetopt.c
 
+.if ${MK_TESTS} != "no"
+SUBDIR+=        tests
+.endif
+
 .include <bsd.prog.mk>

usr.bin/pr/pr.c

@@ -103,7 +103,7 @@ static char	schar;		/* text column separation character */
 static int	sflag;		/* -s option for multiple columns */
 static int	nohead;		/* do not write head and trailer */
 static int	pgwd;		/* page width with multiple col output */
-static const char *timefrmt;	/* time conversion string */
+static char	*timefrmt;	/* time conversion string */
 
 /*
  * misc globals
@@ -135,6 +135,7 @@ main(int argc, char *argv[])
 			ret_val = horzcol(argc, argv);
 		else
 			ret_val = vertcol(argc, argv);
+		free(timefrmt);
 	} else
 		usage();
 	flsh_errs();
@@ -207,6 +208,7 @@ onecol(int argc, char *argv[])
 	 * allocate header buffer
 	 */
 	if ((hbuf = malloc((unsigned)(HDBUF + offst)*sizeof(char))) == NULL) {
+		free(obuf);
 		mfail();
 		return(1);
 	}
@@ -259,7 +261,7 @@ onecol(int argc, char *argv[])
 					break;
 				if (!linecnt && !nohead &&
 					prhead(hbuf, fname, pagecnt))
-					return(1);
+					goto err;
 
 				/*
 				 * start of new line.
@@ -268,9 +270,9 @@ onecol(int argc, char *argv[])
 					if (num)
 						addnum(nbuf, num, ++lncnt);
 					if (otln(obuf,cnt+off, &ips, &ops, mor))
-						return(1);
+						goto err;
 				} else if (otln(lbuf, cnt, &ips, &ops, mor))
-					return(1);
+					goto err;
 
 				/*
 				 * if line bigger than buffer, get more
@@ -293,7 +295,7 @@ onecol(int argc, char *argv[])
 			 * fill to end of page
 			 */
 			if (linecnt && prtail(lines-linecnt-lrgln, lrgln))
-				return(1);
+				goto err;
 
 			/*
 			 * On EOF go to next file
@@ -306,8 +308,14 @@ onecol(int argc, char *argv[])
 			(void)fclose(inf);
 	}
 	if (eoptind < argc)
-		return(1);
+		goto err;
+	free(hbuf);
+	free(obuf);
 	return(0);
+err:
+	free(hbuf);
+	free(obuf);
+	return(1);
 }
 
 /*
@@ -317,27 +325,27 @@ int
 vertcol(int argc, char *argv[])
 {
 	char *ptbf;
-	char **lstdat;
+	char **lstdat = NULL;
 	int i;
 	int j;
 	int cnt = -1;
 	int pln;
-	int *indy;
+	int *indy = NULL;
 	int cvc;
-	int *lindy;
+	int *lindy = NULL;
 	int lncnt;
 	int stp;
 	int pagecnt;
 	int col = colwd + 1;
 	int mxlen = pgwd + offst + 1;
 	int mclcnt = clcnt - 1;
-	struct vcol *vc;
+	struct vcol *vc = NULL;
 	int mvc;
 	int tvc;
 	int cw = nmwd + 1;
 	int fullcol;
-	char *buf;
-	char *hbuf;
+	char *buf = NULL;
+	char *hbuf = NULL;
 	char *ohbuf;
 	const char *fname;
 	FILE *inf;
@@ -345,6 +353,7 @@ vertcol(int argc, char *argv[])
 	int cps = 0;
 	int ops = 0;
 	int mor = 0;
+	int retval = 1;
 
 	/*
 	 * allocate page buffer
@@ -359,7 +368,7 @@ vertcol(int argc, char *argv[])
 	 */
 	if ((hbuf = malloc((unsigned)(HDBUF + offst)*sizeof(char))) == NULL) {
 		mfail();
-		return(1);
+		goto out;
 	}
 	ohbuf = hbuf + offst;
 	if (offst)
@@ -372,7 +381,7 @@ vertcol(int argc, char *argv[])
 	if ((vc =
 	    (struct vcol *)malloc((unsigned)mvc*sizeof(struct vcol))) == NULL) {
 		mfail();
-		return(1);
+		goto out;
 	}
 
 	/*
@@ -380,7 +389,7 @@ vertcol(int argc, char *argv[])
 	 */
 	if ((lstdat = (char **)malloc((unsigned)lines*sizeof(char *))) == NULL){
 		mfail();
-		return(1);
+		goto out;
 	}
 
 	/*
@@ -388,11 +397,11 @@ vertcol(int argc, char *argv[])
 	 */
 	if ((indy = (int *)malloc((unsigned)lines*sizeof(int))) == NULL) {
 		mfail();
-		return(1);
+		goto out;
 	}
 	if ((lindy = (int *)malloc((unsigned)lines*sizeof(int))) == NULL) {
 		mfail();
-		return(1);
+		goto out;
 	}
 
 	if (nmwd)
@@ -533,12 +542,13 @@ vertcol(int argc, char *argv[])
 				 * print header
 				 */
 				if (!nohead && prhead(hbuf, fname, pagecnt))
-					return(1);
+					goto out;
 				for (i = 0; i < pln; ++i) {
 					ips = 0;
 					ops = 0;
-					if (offst&& otln(buf,offst,&ips,&ops,1))
-						return(1);
+					if (offst &&
+					    otln(buf,offst,&ips,&ops,1))
+						goto out;
 					tvc = i;
 
 					for (j = 0; j < clcnt; ++j) {
@@ -563,7 +573,7 @@ vertcol(int argc, char *argv[])
 							cnt = fullcol;
 						if (otln(vc[tvc].pt, cnt, &ips,
 								&ops, 1))
-							return(1);
+							goto out;
 						tvc += pln;
 						if (tvc >= cvc)
 							break;
@@ -572,13 +582,13 @@ vertcol(int argc, char *argv[])
 					 * terminate line
 					 */
 					if (otln(buf, 0, &ips, &ops, 0))
-						return(1);
+						goto out;
 				}
 				/*
 				 * pad to end of page
 				 */
 				if (prtail((lines - pln), 0))
-					return(1);
+					goto out;
 				/*
 				 * done with output, go to next file
 				 */
@@ -597,7 +607,7 @@ vertcol(int argc, char *argv[])
 			 * print header
 			 */
 			if (pln && !nohead && prhead(hbuf, fname, pagecnt))
-				return(1);
+				goto out;
 
 			/*
 			 * output each line
@@ -607,14 +617,14 @@ vertcol(int argc, char *argv[])
 				if ((j = lstdat[i] - ptbf) <= offst)
 					break;
 				if (otln(ptbf, j, &ips, &ops, 0))
-					return(1);
+					goto out;
 			}
 
 			/*
 			 * pad to end of page
 			 */
 			if (pln && prtail((lines - pln), 0))
-				return(1);
+				goto out;
 
 			/*
 			 * if EOF go to next file
@@ -627,8 +637,16 @@ vertcol(int argc, char *argv[])
 			(void)fclose(inf);
 	}
 	if (eoptind < argc)
-		return(1);
-	return(0);
+		goto out;
+	retval = 0;
+out:
+	free(lindy);
+	free(indy);
+	free(lstdat);
+	free(vc);
+	free(hbuf);
+	free(buf);
+	return(retval);
 }
 
 /*
@@ -665,6 +683,7 @@ horzcol(int argc, char *argv[])
 	 * page header
 	 */
 	if ((hbuf = malloc((unsigned)(HDBUF + offst)*sizeof(char))) == NULL) {
+		free(buf);
 		mfail();
 		return(1);
 	}
@@ -744,19 +763,19 @@ horzcol(int argc, char *argv[])
 					break;
 				if (!i && !nohead &&
 					prhead(hbuf, fname, pagecnt))
-					return(1);
+					goto err;
 				/*
 				 * output line
 				 */
 				if (otln(buf, j, &ips, &ops, 0))
-					return(1);
+					goto err;
 			}
 
 			/*
 			 * pad to end of page
 			 */
 			if (i && prtail(lines-i, 0))
-				return(1);
+				goto err;
 
 			/*
 			 * if EOF go to next file
@@ -769,8 +788,14 @@ horzcol(int argc, char *argv[])
 			(void)fclose(inf);
 	}
 	if (eoptind < argc)
-		return(1);
+		goto err;
+	free(hbuf);
+	free(buf);
 	return(0);
+err:
+	free(hbuf);
+	free(buf);
+	return(1);
 }
 
 /*
@@ -786,27 +811,28 @@ mulfile(int argc, char *argv[])
 	int cnt;
 	char *lstdat;
 	int i;
-	FILE **fbuf;
+	FILE **fbuf = NULL;
 	int actf;
 	int lncnt;
 	int col;
 	int pagecnt;
 	int fproc;
-	char *buf;
-	char *hbuf;
+	char *buf = NULL;
+	char *hbuf = NULL;
 	char *ohbuf;
 	const char *fname;
 	int ips = 0;
 	int cps = 0;
 	int ops = 0;
 	int mor = 0;
+	int retval = 1;
 
 	/*
 	 * array of FILE *, one for each operand
 	 */
 	if ((fbuf = (FILE **)malloc((unsigned)clcnt*sizeof(FILE *))) == NULL) {
 		mfail();
-		return(1);
+		goto out;
 	}
 
 	/*
@@ -814,7 +840,7 @@ mulfile(int argc, char *argv[])
 	 */
 	if ((hbuf = malloc((unsigned)(HDBUF + offst)*sizeof(char))) == NULL) {
 		mfail();
-		return(1);
+		goto out;
 	}
 	ohbuf = hbuf + offst;
 
@@ -838,7 +864,7 @@ mulfile(int argc, char *argv[])
 	 * if no files, exit
 	 */
 	if (!j)
-		return(1);
+		goto out;
 
 	/*
 	 * calculate page boundaries based on open file count
@@ -854,7 +880,7 @@ mulfile(int argc, char *argv[])
 	if (colwd < 1) {
 		(void)fprintf(err,
 		  "pr: page width too small for %d columns\n", clcnt);
-		return(1);
+		goto out;
 	}
 	actf = clcnt;
 	col = colwd + 1;
@@ -864,7 +890,7 @@ mulfile(int argc, char *argv[])
 	 */
 	if ((buf = malloc((unsigned)(pgwd+offst+1)*sizeof(char))) == NULL) {
 		mfail();
-		return(1);
+		goto out;
 	}
 	if (offst) {
 		(void)memset(buf, (int)' ', offst);
@@ -951,13 +977,13 @@ mulfile(int argc, char *argv[])
 				break;
 
 			if (!i && !nohead && prhead(hbuf, fname, pagecnt))
-				return(1);
+				goto out;
 
 			/*
 			 * output line
 			 */
 			if (otln(buf, j, &ips, &ops, 0))
-				return(1);
+				goto out;
 
 			/*
 			 * if no more active files, done
@@ -972,12 +998,17 @@ mulfile(int argc, char *argv[])
 		 * pad to end of page
 		 */
 		if (i && prtail(lines-i, 0))
-			return(1);
+			goto out;
 		++pagecnt;
 	}
 	if (eoptind < argc)
-		return(1);
-	return(0);
+		goto out;
+	retval = 0;
+out:
+	free(buf);
+	free(hbuf);
+	free(fbuf);
+	return(retval);
 }
 
 /*
@@ -1344,6 +1375,7 @@ nxtfile(int argc, char **argv, const char **fname, char *buf, int dt)
 					(void)fprintf(err,
 					     "pr: cannot get time of day, %s\n",
 					     strerror(errno));
+					fclose(inf);
 					return(NULL);
 				}
 				timeptr = localtime(&tv_sec);
@@ -1354,6 +1386,7 @@ nxtfile(int argc, char **argv, const char **fname, char *buf, int dt)
 					(void)fprintf(err,
 						"pr: cannot stat %s, %s\n",
 						argv[eoptind], strerror(errno));
+					fclose(inf);
 					return(NULL);
 				}
 				timeptr = localtime(&(statbuf.st_mtime));
@@ -1725,7 +1758,9 @@ setup(int argc, char *argv[])
 			break;
 		case 'w':
 			++wflag;
-			if (!isdigit((unsigned char)*eoptarg) || ((pgwd = atoi(eoptarg)) < 1)){
+			if ((eoptarg == NULL ) ||
+			    !isdigit((unsigned char)*eoptarg) ||
+			    ((pgwd = atoi(eoptarg)) < 1)){
 				(void)fputs(
 				   "pr: -w width must be 1 or more \n",err);
 				return(1);

usr.bin/pr/tests/Makefile

@@ -0,0 +1,17 @@
+# $FreeBSD$
+
+PACKAGE=	tests
+
+ATF_TESTS_SH+=    	basic2_test
+NETBSD_ATF_TESTS_SH=    basic
+
+${PACKAGE}FILES+=	across.out
+${PACKAGE}FILES+=	d_basic.in
+${PACKAGE}FILES+=	d_basic.out
+${PACKAGE}FILES+=	merge.out
+${PACKAGE}FILES+=	other.in
+${PACKAGE}FILES+=	threecol.out
+
+.include <netbsd-tests.test.mk>
+
+.include <bsd.test.mk>

usr.bin/pr/tests/across.out

@@ -0,0 +1,2 @@
+987 654 321			    ghi def abc
+foo bar baz

usr.bin/pr/tests/basic2_test.sh

@@ -0,0 +1,59 @@
+# Copyright (c) 2017 Alan Somers
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+#    notice, this list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright
+#    notice, this list of conditions and the following disclaimer in the
+#    documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
+# ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+# TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+# PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
+# BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+# POSSIBILITY OF SUCH DAMAGE.
+#
+# $FreeBSD$
+
+atf_test_case across
+across_head() {
+        atf_set "descr" "Format columns in round-robin order with pr -a"
+}
+across_body() {
+	atf_check -s exit:0 -o file:$(atf_get_srcdir)/across.out \
+		-x "pr -t -a -2 $(atf_get_srcdir)/other.in"
+}
+
+atf_test_case merge
+merge_head() {
+        atf_set "descr" "Merge two files with pr -m"
+}
+merge_body() {
+	atf_check -s ignore -o file:$(atf_get_srcdir)/merge.out \
+		pr -t -m $(atf_get_srcdir)/d_basic.in $(atf_get_srcdir)/other.in
+}
+
+atf_test_case threecol
+threecol_head() {
+        atf_set "descr" "Format a file with three columns"
+}
+threecol_body() {
+	atf_check -s ignore -o file:$(atf_get_srcdir)/threecol.out \
+		pr -t -3 $(atf_get_srcdir)/other.in
+}
+
+atf_init_test_cases()
+{
+        atf_add_test_case across
+        atf_add_test_case merge
+        atf_add_test_case threecol
+}

usr.bin/pr/tests/merge.out

@@ -0,0 +1,3 @@
+123 456 789			    987 654 321
+abc def ghi			    ghi def abc
+				    foo bar baz

usr.bin/pr/tests/other.in

@@ -0,0 +1,3 @@
+987 654 321
+ghi def abc
+foo bar baz

usr.bin/pr/tests/threecol.out

@@ -0,0 +1 @@
+987 654 321		ghi def abc		foo bar baz