trie: Don't assume that chars are unsigned < 126 (#386)

* trie: Don't assume that chars are unsigned < 126 Trie fails on systems with unsigned chars when using characters over 126.
ClusterLabs · Mar 9, 2020 · 1daca57 · 1daca57
1 parent 9249caf
commit 1daca57
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 2 deletions.
diff --git a/lib/trie.c b/lib/trie.c
@@ -64,8 +64,8 @@ static void trie_destroy_node(struct trie_node *node);
  * characters are stored in reverse to make accessing the
  * more common case (non-control chars) more space efficient.
  */
-#define TRIE_CHAR2INDEX(ch) (126 - ch)
-#define TRIE_INDEX2CHAR(idx) (126 - idx)
+#define TRIE_CHAR2INDEX(ch) (127 - (signed char)ch)
+#define TRIE_INDEX2CHAR(idx) (127 - (signed char)idx)
 
 
 static int32_t

diff --git a/tests/check_map.c b/tests/check_map.c
@@ -41,6 +41,21 @@ const char *chars2[] = {
 	NULL,
 };
 
+const char *composers[] = {
+	"Béla Bartók",
+	"Zoltán Kodály",
+	"Ludwig van Beethoven",
+	"Wolfgang Amadeus Mozart",
+	"Leoš Janáček",
+	"Benjamin Britten",
+	"Josef Haydn",
+	"Claude Debussy",
+	"Charles Ives",
+	/* Maybe in an alien language ... but they can cause trie crashes & conflicts */
+	"\x7e\x7f\x80\x81", "\x7e", "\x7e\x7f", "\x7e\x7f\x80",
+};
+
+
 static char *notified_key = NULL;
 static void *notified_value = NULL;
 static void *notified_new_value = NULL;
@@ -717,6 +732,45 @@ test_map_load(qb_map_t *m, const char* test_name)
 	qb_log(LOG_INFO, "%25s %12.2f dels/sec (%d/%fs)\n", test_name, ops, count2, secs);
 }
 
+static void test_accents_load(qb_map_t *m, const char* test_name)
+{
+	int i;
+	int32_t res = 0;
+	int32_t count = 0;
+	int32_t count2;
+	void *value;
+
+	ck_assert(m != NULL);
+	/*
+	 * Load accented names
+	 */
+	for (i=0; i<sizeof(composers)/sizeof(char*); i++) {
+		qb_map_put(m, strdup(composers[i]), strdup(composers[i]));
+		count++;
+	}
+	ck_assert_int_eq(qb_map_count_get(m), count);
+
+	/*
+	 * Verify dictionary produces correct values
+	 */
+	count2 = 0;
+	for (i=0; i<sizeof(composers)/sizeof(char*); i++) {
+		value = qb_map_get(m, composers[i]);
+		ck_assert_str_eq(value, composers[i]);
+		count2++;
+	}
+	ck_assert_int_eq(qb_map_count_get(m), count2);
+
+	/*
+	 * Delete all dictionary entries
+	 */
+	for (i=0; i<sizeof(composers)/sizeof(char*); i++) {
+		res = qb_map_rm(m, composers[i]);
+		ck_assert_int_eq(res, QB_TRUE);
+	}
+	ck_assert_int_eq(qb_map_count_get(m), 0);
+}
+
 START_TEST(test_skiplist_simple)
 {
 	qb_map_t *m = qb_skiplist_create();
@@ -873,6 +927,31 @@ START_TEST(test_trie_load)
 }
 END_TEST
 
+START_TEST(test_skiplist_accents)
+{
+	qb_map_t *m;
+	m = qb_skiplist_create();
+	test_accents_load(m, __func__);
+}
+END_TEST
+
+START_TEST(test_hashtable_accents)
+{
+	qb_map_t *m;
+	m = qb_hashtable_create(16);
+	test_accents_load(m, __func__);
+}
+END_TEST
+
+START_TEST(test_trie_accents)
+{
+	qb_map_t *m;
+	m = qb_trie_create();
+	test_accents_load(m, __func__);
+}
+END_TEST
+
+
 /*
  * From Honza: https://github.com/asalkeld/libqb/issues/44
  */
@@ -936,6 +1015,10 @@ map_suite(void)
 	add_tcase(s, tc, test_hashtable_load, 30);
 	add_tcase(s, tc, test_trie_load, 30);
 
+	add_tcase(s, tc, test_skiplist_accents);
+	add_tcase(s, tc, test_hashtable_accents);
+	add_tcase(s, tc, test_trie_accents);
+
 	return s;
 }