always check the result of rb3_insert(), i.e. detect collisions in RB treesHEADmaster-fmsbw-cimaster

This was already done for the goto table, but by first looking up
whether a label already exists in the table.
Since the insert operation already reports entries with the same key
in the tree, we should rather check that. This saves one goto table
lookup per label definition, but on the other hand results in excess
allocations in case of encountering a label twice.

In teco_help_set(), we did not check for collisions at all.
Even thouch they are very unlikely, we could encounter them if
a topic is defined on exactly the same position as in
.SCITECO_TOPIC foo
.SCITECO_TOPIC foo
In this case, we leaked memory.

2 files changed, 15 insertions, 7 deletions

diff --git a/src/goto.c b/src/goto.c
index 42eb964..36002e6 100644
--- a/src/goto.c
+++ b/src/goto.c
@@ -167,12 +167,14 @@ teco_goto_table_find(teco_goto_table_t *ctx, const gchar *name, gsize len)
 gssize
 teco_goto_table_set(teco_goto_table_t *ctx, const gchar *name, gsize len, gsize pc)
 {
-	gssize existing_pc = teco_goto_table_find(ctx, name, len);
-	if (existing_pc >= 0)
-		return existing_pc;
+	teco_goto_label_t *label, *found;
 
-	teco_goto_label_t *label = teco_goto_label_new(name, len, pc);
-	teco_rb3str_insert(&ctx->tree, TRUE, &label->head);
+	label = teco_goto_label_new(name, len, pc);
+	found = (teco_goto_label_t *)teco_rb3str_insert(&ctx->tree, TRUE, &label->head);
+	if (found) {
+		teco_goto_label_free(label);
+		return found->pc;
+	}
 
 #ifdef DEBUG
 	teco_goto_table_dump(ctx);
diff --git a/src/help.c b/src/help.c
index e7e674c..1e4a766 100644
--- a/src/help.c
+++ b/src/help.c
@@ -225,7 +225,7 @@ teco_help_find(const gchar *topic_name)
  * Alternatively, we'd have to keep lists in each teco_help_topic_t.
  */
 static gint
-teco_help_casecmp(const teco_help_topic_t *topic, const teco_help_topic_t *data)
+teco_help_topic_casecmp(const teco_help_topic_t *topic, const teco_help_topic_t *data)
 {
 	return teco_string_casecmp(topic->head.key, data->head.key.data, data->head.key.len)
 		? : strcasecmp(topic->filename, data->filename)
@@ -240,7 +240,13 @@ static inline void
 teco_help_set(const gchar *topic_name, const gchar *filename, gsize pos)
 {
 	teco_help_topic_t *topic = teco_help_topic_new(topic_name, filename, pos);
-	rb3_insert(&teco_help.tree, &topic->head.head, (rb3_cmp *)teco_help_casecmp, topic);
+	if (G_UNLIKELY(rb3_insert(&teco_help.tree, &topic->head.head,
+	                          (rb3_cmp *)teco_help_topic_casecmp, topic) != NULL))
+		/*
+		 * Should not happen - just keep the old entry.
+		 * This will "leak" some memory in the teco_help.chunk, though.
+		 */
+		teco_help_topic_free(topic);
 }
 
 gboolean