It was very surprising that a function called mparse_readfd()

closed the file descriptor passed to it after completing its work,
in particular considering the fact that it required its callers
to call open(2) or mparse_open() beforehand.

Change mparse_readfd() to not call close(2) and change the callers
to call close(2) afterwards, more or less bringing open and close
to the same level of the code and making review easier.  Note that
man.cgi(8) already did that, even though it was wrong in the past.

Small restructuring suggested by Christos Zoulas (NetBSD).

5 files changed, 17 insertions, 11 deletions

diff --git a/demandoc.c b/demandoc.c
index 2e4e932e..f55109cd 100644
--- a/demandoc.c
+++ b/demandoc.c
@@ -1,4 +1,4 @@
-/*	$Id: demandoc.c,v 1.24 2015/10/22 22:06:43 schwarze Exp $ */
+/*	$Id: demandoc.c,v 1.25 2016/01/08 02:13:39 schwarze Exp $ */
 /*
  * Copyright (c) 2011 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -113,6 +113,7 @@ pmandoc(struct mparse *mp, int fd, const char *fn, int list)
 	int		 line, col;
 
 	mparse_readfd(mp, fd, fn);
+	close(fd);
 	mparse_result(mp, &man, NULL);
 	line = 1;
 	col = 0;
diff --git a/main.c b/main.c
index a311eb93..ccebdb4d 100644
--- a/main.c
+++ b/main.c
@@ -1,4 +1,4 @@
-/*	$Id: main.c,v 1.260 2015/12/15 17:38:45 schwarze Exp $ */
+/*	$Id: main.c,v 1.261 2016/01/08 02:13:39 schwarze Exp $ */
 /*
  * Copyright (c) 2008-2012 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2010-2012, 2014, 2015 Ingo Schwarze <schwarze@openbsd.org>
@@ -724,9 +724,11 @@ parse(struct curparse *curp, int fd, const char *file)
 	/* Begin by parsing the file itself. */
 
 	assert(file);
-	assert(fd >= -1);
+	assert(fd > 0);
 
 	rctmp = mparse_readfd(curp->mp, fd, file);
+	if (fd != STDIN_FILENO)
+		close(fd);
 	if (rc < rctmp)
 		rc = rctmp;
 
diff --git a/mandoc.3 b/mandoc.3
index 9af832cc..4e04f854 100644
--- a/mandoc.3
+++ b/mandoc.3
@@ -1,4 +1,4 @@
-.\"	$Id: mandoc.3,v 1.33 2015/10/13 22:59:54 schwarze Exp $
+.\"	$Id: mandoc.3,v 1.34 2016/01/08 02:13:39 schwarze Exp $
 .\"
 .\" Copyright (c) 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
 .\" Copyright (c) 2010, 2013, 2014, 2015 Ingo Schwarze <schwarze@openbsd.org>
@@ -15,7 +15,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: October 13 2015 $
+.Dd $Mdocdate: January 8 2016 $
 .Dt MANDOC 3
 .Os
 .Sh NAME
@@ -177,6 +177,9 @@ or
 parse it with
 .Fn mparse_readfd ;
 .It
+close it with
+.Xr close 2 ;
+.It
 retrieve the syntax tree with
 .Fn mparse_result ;
 .It
@@ -403,6 +406,8 @@ or
 Pass the associated filename in
 .Va fname .
 This function may be called multiple times with different parameters; however,
+.Xr close 2
+and
 .Fn mparse_reset
 should be invoked between parses.
 Declared in
diff --git a/mandocdb.c b/mandocdb.c
index a86a7856..d3701d6f 100644
--- a/mandocdb.c
+++ b/mandocdb.c
@@ -1,4 +1,4 @@
-/*	$Id: mandocdb.c,v 1.211 2016/01/04 14:44:57 schwarze Exp $ */
+/*	$Id: mandocdb.c,v 1.212 2016/01/08 02:13:39 schwarze Exp $ */
 /*
  * Copyright (c) 2011, 2012 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2011-2016 Ingo Schwarze <schwarze@openbsd.org>
@@ -1146,6 +1146,7 @@ mpages_merge(struct mparse *mp)
 		 */
 		if (mlink->dform != FORM_CAT || mlink->fform != FORM_CAT) {
 			mparse_readfd(mp, fd, mlink->file);
+			close(fd);
 			mparse_result(mp, &man, &sodest);
 		}
 
diff --git a/read.c b/read.c
index d1ffb578..52997bb0 100644
--- a/read.c
+++ b/read.c
@@ -1,4 +1,4 @@
-/*	$Id: read.c,v 1.146 2015/11/07 14:22:29 schwarze Exp $ */
+/*	$Id: read.c,v 1.147 2016/01/08 02:13:39 schwarze Exp $ */
 /*
  * Copyright (c) 2008, 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2010-2015 Ingo Schwarze <schwarze@openbsd.org>
@@ -542,6 +542,7 @@ rerun:
 			if (mparse_open(curp, &fd, ln.buf + of) ==
 			    MANDOCLEVEL_OK) {
 				mparse_readfd(curp, fd, ln.buf + of);
+				close(fd);
 				curp->file = save_file;
 			} else {
 				curp->file = save_file;
@@ -773,10 +774,6 @@ mparse_readfd(struct mparse *curp, int fd, const char *file)
 #endif
 			free(blk.buf);
 	}
-
-	if (fd != STDIN_FILENO && close(fd) == -1)
-		perror(file);
-
 	return curp->file_status;
 }