Remove the hack of scrolling forward and backward with +G1G that

many (jmc@, millert@, espie@, deraadt@) considered revolting.
Instead, when using a pager, since we are using a temporary file
for tags anyway, use another temporary file for the formatted
page(s), as suggested by millert@ and similar to what the traditional
BSD man(1) did, except that we use only one single temporary output
file rather than one for each formatted manual page, such that
searching (both with / and :t) works across all the displayed files.

3 files changed, 99 insertions, 84 deletions

diff --git a/usr.bin/mandoc/main.c b/usr.bin/mandoc/main.c
index 28b4d7b465d..76694b39c58 100644
--- a/usr.bin/mandoc/main.c
+++ b/usr.bin/mandoc/main.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: main.c,v 1.148 2015/07/21 03:26:02 schwarze Exp $ */
+/*	$OpenBSD: main.c,v 1.149 2015/07/28 18:38:05 schwarze Exp $ */
 /*
  * Copyright (c) 2008-2012 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2010-2012, 2014, 2015 Ingo Schwarze <schwarze@openbsd.org>
@@ -88,7 +88,6 @@ static	int		  fs_lookup(const struct manpaths *,
 static	void		  fs_search(const struct mansearch *,
 				const struct manpaths *, int, char**,
 				struct manpage **, size_t *);
-static	void		  handle_sigpipe(int);
 static	int		  koptions(int *, char *);
 int			  mandocdb(int, char**);
 static	int		  moptions(int *, char *);
@@ -96,7 +95,7 @@ static	void		  mmsg(enum mandocerr, enum mandoclevel,
 				const char *, int, int, const char *);
 static	void		  parse(struct curparse *, int, const char *);
 static	void		  passthrough(const char *, int, int);
-static	pid_t		  spawn_pager(void);
+static	pid_t		  spawn_pager(struct tag_files *);
 static	int		  toptions(struct curparse *, char *);
 static	void		  usage(enum argmode) __attribute__((noreturn));
 static	int		  woptions(struct curparse *, char *);
@@ -114,6 +113,7 @@ main(int argc, char *argv[])
 	struct manconf	 conf;
 	struct curparse	 curp;
 	struct mansearch search;
+	struct tag_files *tag_files;
 	char		*auxpaths;
 	char		*defos;
 	unsigned char	*uc;
@@ -127,8 +127,8 @@ main(int argc, char *argv[])
 	int		 fd;
 	int		 show_usage;
 	int		 options;
+	int		 use_pager;
 	int		 c;
-	pid_t		 pager_pid;  /* 0: don't use; 1: not yet spawned. */
 
 	if (argc < 1)
 		progname = "mandoc";
@@ -170,7 +170,8 @@ main(int argc, char *argv[])
 	options = MPARSE_SO | MPARSE_UTF8 | MPARSE_LATIN1;
 	defos = NULL;
 
-	pager_pid = 1;
+	use_pager = 1;
+	tag_files = NULL;
 	show_usage = 0;
 	outmode = OUTMODE_DEF;
 
@@ -184,14 +185,14 @@ main(int argc, char *argv[])
 			conf_file = optarg;
 			break;
 		case 'c':
-			pager_pid = 0;
+			use_pager = 0;
 			break;
 		case 'f':
 			search.argmode = ARG_WORD;
 			break;
 		case 'h':
 			conf.output.synopsisonly = 1;
-			pager_pid = 0;
+			use_pager = 0;
 			outmode = OUTMODE_ALL;
 			break;
 		case 'I':
@@ -267,7 +268,7 @@ main(int argc, char *argv[])
 		switch (search.argmode) {
 		case ARG_FILE:
 			outmode = OUTMODE_ALL;
-			pager_pid = 0;
+			use_pager = 0;
 			break;
 		case ARG_NAME:
 			outmode = OUTMODE_ONE;
@@ -398,8 +399,8 @@ main(int argc, char *argv[])
 	if (search.argmode == ARG_FILE && ! moptions(&options, auxpaths))
 		return((int)MANDOCLEVEL_BADARG);
 
-	if (pager_pid == 1 && isatty(STDOUT_FILENO) == 0)
-		pager_pid = 0;
+	if (use_pager && ! isatty(STDOUT_FILENO))
+		use_pager = 0;
 
 	curp.mchars = mchars_alloc();
 	curp.mp = mparse_alloc(options, curp.wlevel, mmsg,
@@ -412,8 +413,8 @@ main(int argc, char *argv[])
 		mparse_keep(curp.mp);
 
 	if (argc < 1) {
-		if (pager_pid == 1)
-			pager_pid = spawn_pager();
+		if (use_pager)
+			tag_files = tag_init();
 		parse(&curp, STDIN_FILENO, "<stdin>");
 	}
 
@@ -424,8 +425,10 @@ main(int argc, char *argv[])
 			rc = rctmp;
 
 		if (fd != -1) {
-			if (pager_pid == 1)
-				pager_pid = spawn_pager();
+			if (use_pager) {
+				tag_files = tag_init();
+				use_pager = 0;
+			}
 
 			if (resp == NULL)
 				parse(&curp, fd, *argv);
@@ -467,15 +470,14 @@ out:
 	free(defos);
 
 	/*
-	 * If a pager is attached, flush the pipe leading to it
-	 * and signal end of file such that the user can browse
-	 * to the end.  Then wait for the user to close the pager.
+	 * When using a pager, finish writing both temporary files,
+	 * fork it, wait for the user to close it, and clean up.
 	 */
 
-	if (pager_pid != 0 && pager_pid != 1) {
+	if (tag_files != NULL) {
 		fclose(stdout);
 		tag_write();
-		waitpid(pager_pid, NULL, 0);
+		waitpid(spawn_pager(tag_files), NULL, 0);
 		tag_unlink();
 	}
 
@@ -922,22 +924,14 @@ mmsg(enum mandocerr t, enum mandoclevel lvl,
 	fputc('\n', stderr);
 }
 
-static void
-handle_sigpipe(int signum)
-{
-
-	exit((int)rc);
-}
-
 static pid_t
-spawn_pager(void)
+spawn_pager(struct tag_files *tag_files)
 {
 #define MAX_PAGER_ARGS 16
 	char		*argv[MAX_PAGER_ARGS];
 	const char	*pager;
 	char		*cp;
 	size_t		 cmdlen;
-	int		 fildes[2];
 	int		 argc;
 	pid_t		 pager_pid;
 
@@ -966,29 +960,18 @@ spawn_pager(void)
 			break;
 	}
 
-	/* Read all text right away and use the tag file. */
+	/* For more(1) and less(1), use the tag file. */
 
-	for (;;) {
-		if ((cmdlen = strlen(argv[0])) < 4)
-			break;
+	if ((cmdlen = strlen(argv[0])) >= 4) {
 		cp = argv[0] + cmdlen - 4;
-		if (strcmp(cp, "less") && strcmp(cp, "more"))
-			break;
-		if ((cp = tag_init()) == NULL)
-			break;
-		argv[argc++] = mandoc_strdup("+G1G");
-		argv[argc++] = mandoc_strdup("-T");
-		argv[argc++] = cp;
-		break;
+		if (strcmp(cp, "less") == 0 || strcmp(cp, "more") == 0) {
+			argv[argc++] = mandoc_strdup("-T");
+			argv[argc++] = tag_files->tfn;
+		}
 	}
+	argv[argc++] = tag_files->ofn;
 	argv[argc] = NULL;
 
-	if (pipe(fildes) == -1) {
-		fprintf(stderr, "%s: pipe: %s\n",
-		    progname, strerror(errno));
-		return(0);
-	}
-
 	switch (pager_pid = fork()) {
 	case -1:
 		fprintf(stderr, "%s: fork: %s\n",
@@ -997,29 +980,17 @@ spawn_pager(void)
 	case 0:
 		break;
 	default:
-		close(fildes[0]);
-		if (dup2(fildes[1], STDOUT_FILENO) == -1) {
-			fprintf(stderr, "%s: dup output: %s\n",
-			    progname, strerror(errno));
-			exit((int)MANDOCLEVEL_SYSERR);
-		}
-		close(fildes[1]);
-		signal(SIGPIPE, handle_sigpipe);
 		return(pager_pid);
 	}
 
 	/* The child process becomes the pager. */
 
-	close(fildes[1]);
-	if (dup2(fildes[0], STDIN_FILENO) == -1) {
-		fprintf(stderr, "%s: dup input: %s\n",
-		    progname, strerror(errno));
+	if (dup2(tag_files->ofd, STDOUT_FILENO) == -1) {
+		fprintf(stderr, "pager: stdout: %s\n", strerror(errno));
 		exit((int)MANDOCLEVEL_SYSERR);
 	}
-	close(fildes[0]);
-
-	/* Hand over to the pager. */
-
+	close(tag_files->ofd);
+	close(tag_files->tfd);
 	execvp(argv[0], argv);
 	fprintf(stderr, "%s: exec %s: %s\n",
 	    progname, argv[0], strerror(errno));
diff --git a/usr.bin/mandoc/tag.c b/usr.bin/mandoc/tag.c
index dd76a02cce0..1bf7cdbdf84 100644
--- a/usr.bin/mandoc/tag.c
+++ b/usr.bin/mandoc/tag.c
@@ -1,4 +1,4 @@
-/*      $OpenBSD: tag.c,v 1.5 2015/07/25 14:28:40 schwarze Exp $    */
+/*      $OpenBSD: tag.c,v 1.6 2015/07/28 18:38:05 schwarze Exp $    */
 /*
  * Copyright (c) 2015 Ingo Schwarze <schwarze@openbsd.org>
  *
@@ -40,29 +40,49 @@ static	void	 tag_free(void *, void *);
 static	void	*tag_calloc(size_t, size_t, void *);
 
 static struct ohash	 tag_data;
-static char		*tag_fn = NULL;
-static int		 tag_fd = -1;
+static struct tag_files	 tag_files;
 
 
 /*
- * Set up the ohash table to collect output line numbers
- * where various marked-up terms are documented and create
- * the temporary tags file, saving the name for the pager.
+ * Prepare for using a pager.
+ * Not all pagers are capable of using a tag file,
+ * but for simplicity, create it anyway.
  */
-char *
+struct tag_files *
 tag_init(void)
 {
 	struct ohash_info	 tag_info;
+	int			 ofd;
 
-	tag_fn = mandoc_strdup("/tmp/man.XXXXXXXXXX");
+	ofd = -1;
+	tag_files.tfd = -1;
+
+	/* Save the original standard output for use by the pager. */
+
+	if ((tag_files.ofd = dup(STDOUT_FILENO)) == -1)
+		goto fail;
+
+	/* Create both temporary output files. */
+
+	(void)strlcpy(tag_files.ofn, "/tmp/man.XXXXXXXXXX",
+	    sizeof(tag_files.ofn));
+	(void)strlcpy(tag_files.tfn, "/tmp/man.XXXXXXXXXX",
+	    sizeof(tag_files.tfn));
 	signal(SIGHUP, tag_signal);
 	signal(SIGINT, tag_signal);
 	signal(SIGTERM, tag_signal);
-	if ((tag_fd = mkstemp(tag_fn)) == -1) {
-		free(tag_fn);
-		tag_fn = NULL;
-		return(NULL);
-	}
+	if ((ofd = mkstemp(tag_files.ofn)) == -1)
+		goto fail;
+	if ((tag_files.tfd = mkstemp(tag_files.tfn)) == -1)
+		goto fail;
+	if (dup2(ofd, STDOUT_FILENO) == -1)
+		goto fail;
+	close(ofd);
+
+	/*
+	 * Set up the ohash table to collect output line numbers
+	 * where various marked-up terms are documented.
+	 */
 
 	tag_info.alloc = tag_alloc;
 	tag_info.calloc = tag_calloc;
@@ -70,7 +90,21 @@ tag_init(void)
 	tag_info.key_offset = offsetof(struct tag_entry, s);
 	tag_info.data = NULL;
 	ohash_init(&tag_data, 4, &tag_info);
-	return(tag_fn);
+	return(&tag_files);
+
+fail:
+	tag_unlink();
+	if (ofd != -1)
+		close(ofd);
+	if (tag_files.ofd != -1)
+		close(tag_files.ofd);
+	if (tag_files.tfd != -1)
+		close(tag_files.tfd);
+	*tag_files.ofn = '\0';
+	*tag_files.tfn = '\0';
+	tag_files.ofd = -1;
+	tag_files.tfd = -1;
+	return(NULL);
 }
 
 /*
@@ -84,7 +118,7 @@ tag_put(const char *s, int prio, size_t line)
 	size_t			 len;
 	unsigned int		 slot;
 
-	if (tag_fd == -1)
+	if (tag_files.tfd <= 0)
 		return;
 	slot = ohash_qlookup(&tag_data, s);
 	entry = ohash_find(&tag_data, slot);
@@ -110,13 +144,14 @@ tag_write(void)
 	struct tag_entry	*entry;
 	unsigned int		 slot;
 
-	if (tag_fd == -1)
+	if (tag_files.tfd <= 0)
 		return;
-	stream = fdopen(tag_fd, "w");
+	stream = fdopen(tag_files.tfd, "w");
 	entry = ohash_first(&tag_data, &slot);
 	while (entry != NULL) {
 		if (stream != NULL)
-			fprintf(stream, "%s - %zu\n", entry->s, entry->line);
+			fprintf(stream, "%s %s %zu\n",
+			    entry->s, tag_files.ofn, entry->line);
 		free(entry);
 		entry = ohash_next(&tag_data, &slot);
 	}
@@ -129,8 +164,10 @@ void
 tag_unlink(void)
 {
 
-	if (tag_fn != NULL)
-		unlink(tag_fn);
+	if (*tag_files.ofn != '\0')
+		unlink(tag_files.ofn);
+	if (*tag_files.tfn != '\0')
+		unlink(tag_files.tfn);
 }
 
 static void
diff --git a/usr.bin/mandoc/tag.h b/usr.bin/mandoc/tag.h
index 4836a720995..0ce9571d904 100644
--- a/usr.bin/mandoc/tag.h
+++ b/usr.bin/mandoc/tag.h
@@ -1,4 +1,4 @@
-/*      $OpenBSD: tag.h,v 1.4 2015/07/25 14:28:40 schwarze Exp $    */
+/*      $OpenBSD: tag.h,v 1.5 2015/07/28 18:38:05 schwarze Exp $    */
 /*
  * Copyright (c) 2015 Ingo Schwarze <schwarze@openbsd.org>
  *
@@ -15,9 +15,16 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+struct	tag_files {
+	char	 ofn[20];
+	char	 tfn[20];
+	int	 ofd;
+	int	 tfd;
+};
+
 __BEGIN_DECLS
 
-char	*tag_init(void);
+struct tag_files *tag_init(void);
 void	 tag_put(const char *, int, size_t);
 void	 tag_write(void);
 void	 tag_unlink(void);