Correctly handle non-unique tags even when NODE_ID and NODE_HREF fall

apart, NODE_ID occurring earlier than NODE_HREF.

3 files changed, 66 insertions, 54 deletions

diff --git a/html.c b/html.c
index 3aef282a..6e18fac2 100644
--- a/html.c
+++ b/html.c
@@ -1,4 +1,4 @@
-/* $Id: html.c,v 1.268 2020/04/18 20:40:10 schwarze Exp $ */
+/* $Id: html.c,v 1.269 2020/04/19 15:16:56 schwarze Exp $ */
 /*
  * Copyright (c) 2011-2015, 2017-2020 Ingo Schwarze <schwarze@openbsd.org>
  * Copyright (c) 2008-2011, 2014 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -112,6 +112,11 @@ static	const struct htmldata htmltags[TAG_MAX] = {
 };
 
 /* Avoid duplicate HTML id= attributes. */
+
+struct	id_entry {
+	int	 ord;	/* Ordinal number of the latest occurrence. */
+	char	 id[];	/* The id= attribute without any ordinal suffix. */
+};
 static	struct ohash	 id_unique;
 
 static	void	 html_reset_internal(struct html *);
@@ -146,7 +151,7 @@ html_alloc(const struct manoutput *outopts)
 	if (outopts->toc)
 		h->oflags |= HTML_TOC;
 
-	mandoc_ohash_init(&id_unique, 4, 0);
+	mandoc_ohash_init(&id_unique, 4, offsetof(struct id_entry, id));
 
 	return h;
 }
@@ -155,17 +160,17 @@ static void
 html_reset_internal(struct html *h)
 {
 	struct tag	*tag;
-	char		*cp;
+	struct id_entry	*entry;
 	unsigned int	 slot;
 
 	while ((tag = h->tag) != NULL) {
 		h->tag = tag->next;
 		free(tag);
 	}
-	cp = ohash_first(&id_unique, &slot);
-	while (cp != NULL) {
-		free(cp);
-		cp = ohash_next(&id_unique, &slot);
+	entry = ohash_first(&id_unique, &slot);
+	while (entry != NULL) {
+		free(entry);
+		entry = ohash_next(&id_unique, &slot);
 	}
 	ohash_delete(&id_unique);
 }
@@ -174,7 +179,7 @@ void
 html_reset(void *p)
 {
 	html_reset_internal(p);
-	mandoc_ohash_init(&id_unique, 4, 0);
+	mandoc_ohash_init(&id_unique, 4, offsetof(struct id_entry, id));
 }
 
 void
@@ -329,20 +334,24 @@ html_fillmode(struct html *h, enum roff_tok want)
  * element and/or as a segment identifier for a URI in an <a> element.
  * The function may fail and return NULL if the node lacks text data
  * to create the attribute from.
- * If the "unique" argument is 0, the caller is responsible for
- * free(3)ing the returned string after using it.
+ * The caller is responsible for free(3)ing the returned string.
+ *
  * If the "unique" argument is non-zero, the "id_unique" ohash table
- * is used for de-duplication and owns the returned string, so the
- * caller must not free(3) it.  In this case, it will be freed
- * automatically by html_reset() or html_free().
+ * is used for de-duplication.  If the "unique" argument is 1,
+ * it is the first time the function is called for this tag and
+ * location, so if an ordinal suffix is needed, it is incremented.
+ * If the "unique" argument is 2, it is the second time the function
+ * is called for this tag and location, so the ordinal suffix
+ * remains unchanged.
  */
 char *
 html_make_id(const struct roff_node *n, int unique)
 {
 	const struct roff_node	*nch;
-	char			*buf, *bufs, *cp;
+	struct id_entry		*entry;
+	char			*buf, *cp;
+	size_t			 len;
 	unsigned int		 slot;
-	int			 suffix;
 
 	if (n->tag != NULL)
 		buf = mandoc_strdup(n->tag);
@@ -386,25 +395,21 @@ html_make_id(const struct roff_node *n, int unique)
 
 	/* Avoid duplicate HTML id= attributes. */
 
-	bufs = NULL;
-	suffix = 1;
 	slot = ohash_qlookup(&id_unique, buf);
-	cp = ohash_find(&id_unique, slot);
-	if (cp != NULL) {
-		while (cp != NULL) {
-			free(bufs);
-			if (++suffix > 127) {
-				free(buf);
-				return NULL;
-			}
-			mandoc_asprintf(&bufs, "%s_%d", buf, suffix);
-			slot = ohash_qlookup(&id_unique, bufs);
-			cp = ohash_find(&id_unique, slot);
-		}
-		free(buf);
-		buf = bufs;
+	if ((entry = ohash_find(&id_unique, slot)) == NULL) {
+		len = strlen(buf) + 1;
+		entry = mandoc_malloc(sizeof(*entry) + len);
+		entry->ord = 1;
+		memcpy(entry->id, buf, len);
+		ohash_insert(&id_unique, slot, entry);
+	} else if (unique == 1)
+		entry->ord++;
+
+	if (entry->ord > 1) {
+		cp = buf;
+		mandoc_asprintf(&buf, "%s_%d", cp, entry->ord);
+		free(cp);
 	}
-	ohash_insert(&id_unique, slot, buf);
 	return buf;
 }
 
@@ -786,7 +791,7 @@ print_otag_id(struct html *h, enum htmltag elemtype, const char *cattr,
 	if (n->flags & NODE_ID)
 		id = html_make_id(n, 1);
 	if (n->flags & NODE_HREF)
-		href = id == NULL ? html_make_id(n, 0) : id;
+		href = id == NULL ? html_make_id(n, 2) : id;
 	if (href != NULL && htmltags[elemtype].flags & HTML_INPHRASE)
 		ret = print_otag(h, TAG_A, "chR", "permalink", href);
 	t = print_otag(h, elemtype, "ci", cattr, id);
@@ -804,6 +809,7 @@ print_otag_id(struct html *h, enum htmltag elemtype, const char *cattr,
 				print_otag(h, TAG_A, "chR", "permalink", href);
 		}
 	}
+	free(id);
 	if (id == NULL)
 		free(href);
 	return ret;
@@ -915,7 +921,7 @@ print_tagged_text(struct html *h, const char *word, struct roff_node *n)
 	print_metaf(h);
 	print_indent(h);
 
-	if (n != NULL && (href = html_make_id(n, 0)) != NULL) {
+	if (n != NULL && (href = html_make_id(n, 2)) != NULL) {
 		t = print_otag(h, TAG_A, "chR", "permalink", href);
 		free(href);
 	} else
diff --git a/mandoc_html.3 b/mandoc_html.3
index bcb8b178..8b9a4420 100644
--- a/mandoc_html.3
+++ b/mandoc_html.3
@@ -1,4 +1,4 @@
-.\"	$Id: mandoc_html.3,v 1.21 2020/04/18 20:44:09 schwarze Exp $
+.\"	$Id: mandoc_html.3,v 1.22 2020/04/19 15:16:56 schwarze Exp $
 .\"
 .\" Copyright (c) 2014, 2017, 2018 Ingo Schwarze <schwarze@openbsd.org>
 .\"
@@ -14,7 +14,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: April 18 2020 $
+.Dd $Mdocdate: April 19 2020 $
 .Dt MANDOC_HTML 3
 .Os
 .Sh NAME
@@ -366,6 +366,18 @@ If the
 .Fa unique
 argument is non-zero, deduplication is performed by appending an
 underscore and a decimal integer, if necessary.
+If the
+.Fa unique
+argument is 1, this is assumed to be the first call for this tag
+at this location, typically for use by
+.Dv NODE_ID ,
+so the integer is incremented before use.
+If the
+.Fa unique
+argument is 2, this is ssumed to be the second call for this tag
+at this location, typically for use by
+.Dv NODE_HREF ,
+so the existing integer, if any, is used without incrementing it.
 .Pp
 The function
 .Fn print_otag_id
@@ -400,9 +412,9 @@ This function is a wrapper around
 .Fn html_make_id
 and
 .Fn print_otag ,
-fixing the
+automatically chosing the
 .Fa unique
-argument to 1 and the
+argument appropriately and setting the
 .Fa fmt
 arguments to
 .Qq chR
@@ -457,20 +469,9 @@ returns a newly allocated string or
 if
 .Fa n
 lacks text data to create the attribute from.
-If the
-.Fa unique
-argument is 0, the caller is responsible for
+The caller is responsible for
 .Xr free 3 Ns ing
 the returned string after using it.
-If the
-.Fa unique
-argument is non-zero, the
-.Va id_unique
-ohash table is used for de-duplication and owns the returned string.
-In this case, it will be freed automatically by
-.Fn html_reset
-or
-.Fn html_free .
 .Pp
 In case of
 .Xr malloc 3
diff --git a/mdoc_html.c b/mdoc_html.c
index bff4757f..d96df6df 100644
--- a/mdoc_html.c
+++ b/mdoc_html.c
@@ -1,4 +1,4 @@
-/* $Id: mdoc_html.c,v 1.339 2020/04/18 20:40:10 schwarze Exp $ */
+/* $Id: mdoc_html.c,v 1.340 2020/04/19 15:16:56 schwarze Exp $ */
 /*
  * Copyright (c) 2014-2020 Ingo Schwarze <schwarze@openbsd.org>
  * Copyright (c) 2008-2011, 2014 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -695,8 +695,10 @@ mdoc_tg_pre(MDOC_ARGS)
 {
 	char	*id;
 
-	if ((id = html_make_id(n, 1)) != NULL)
+	if ((id = html_make_id(n, 1)) != NULL) {
 		print_tagq(h, print_otag(h, TAG_MARK, "i", id));
+		free(id);
+	}
 	return 0;
 }
 
@@ -1211,6 +1213,8 @@ mdoc_skip_pre(MDOC_ARGS)
 static int
 mdoc_pp_pre(MDOC_ARGS)
 {
+	char	*id;
+
 	if (n->flags & NODE_NOFILL) {
 		print_endline(h);
 		if (n->flags & NODE_ID)
@@ -1221,8 +1225,9 @@ mdoc_pp_pre(MDOC_ARGS)
 		}
 	} else {
 		html_close_paragraph(h);
-		print_otag(h, TAG_P, "ci", "Pp",
-		    n->flags & NODE_ID ? html_make_id(n, 1) : NULL);
+		id = n->flags & NODE_ID ? html_make_id(n, 1) : NULL;
+		print_otag(h, TAG_P, "ci", "Pp", id);
+		free(id);
 	}
 	return 0;
 }