I forgot to attach the patches.
Again the first one is acked by me, the second
needs a review.
Michal
On 10/05/2016 01:45 PM, Michal Židek wrote:
ACK to the code from Philip. I amended the commit
message to meet our style.
I would like to push this together with at least some
sanity tests. See the second patch. I am looking for
someone from SSSD developers to review the tests.
Michal
PS: Btw. Philip, I am interested for what project are you
using ding-libs.
On 09/30/2016 11:55 PM, Philip Prindeville wrote:
From: Philip Prindeville <phil...@fedoraproject.org>
Add c_str as a "const char *" to the hash_key_t.
Redux per comments from Michal Zidek.
---
dhash/dhash.c | 29 +++++++++++++++++++++--------
dhash/dhash.h | 4 +++-
dhash/examples/dhash_test.c | 3 +++
3 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/dhash/dhash.c b/dhash/dhash.c
index 45ee0cf..98439e8 100644
--- a/dhash/dhash.c
+++ b/dhash/dhash.c
@@ -176,7 +176,7 @@ static void sys_free_wrapper(void *ptr, void *pvt)
static address_t convert_key(hash_key_t *key)
{
address_t h;
- unsigned char *k;
+ const unsigned char *k;
switch(key->type) {
case HASH_KEY_ULONG:
@@ -184,7 +184,12 @@ static address_t convert_key(hash_key_t *key)
break;
case HASH_KEY_STRING:
/* Convert string to integer */
- for (h = 0, k = (unsigned char *) key->str; *k; k++)
+ for (h = 0, k = (const unsigned char *) key->str; *k; k++)
+ h = h * PRIME_1 ^ (*k - ' ');
+ break;
+ case HASH_KEY_CONST_STRING:
+ /* Convert string to integer */
+ for (h = 0, k = (const unsigned char *) key->c_str; *k; k++)
h = h * PRIME_1 ^ (*k - ' ');
break;
default:
@@ -212,6 +217,7 @@ static bool is_valid_key_type(hash_key_enum key_type)
switch(key_type) {
case HASH_KEY_ULONG:
case HASH_KEY_STRING:
+ case HASH_KEY_CONST_STRING:
return true;
default:
return false;
@@ -244,6 +250,8 @@ static bool key_equal(hash_key_t *a, hash_key_t *b)
return (a->ul == b->ul);
case HASH_KEY_STRING:
return (strcmp(a->str, b->str) == 0);
+ case HASH_KEY_CONST_STRING:
+ return (strcmp(a->c_str, b->c_str) == 0);
}
return false;
}
@@ -670,8 +678,11 @@ int hash_destroy(hash_table_t *table)
while (p != NULL) {
q = p->next;
hdelete_callback(table,
HASH_TABLE_DESTROY, &p->entry);
- if (p->entry.key.type == HASH_KEY_STRING) {
- hfree(table, (char *)p->entry.key.str);
+ if (p->entry.key.type == HASH_KEY_STRING
+ || p->entry.key.type ==
HASH_KEY_CONST_STRING) {
+ /* Internally we do not use constant
memory for keys
+ * in hash table elements. */
+ hfree(table, p->entry.key.str);
}
hfree(table, (char *)p);
p = q;
@@ -972,13 +983,14 @@ int hash_enter(hash_table_t *table, hash_key_t
*key, hash_value_t *value)
element->entry.key.ul = key->ul;
break;
case HASH_KEY_STRING:
- len = strlen(key->str)+1;
+ case HASH_KEY_CONST_STRING:
+ len = strlen(key->c_str) + 1;
element->entry.key.str = halloc(table, len);
if (element->entry.key.str == NULL) {
hfree(table, element);
return HASH_ERROR_NO_MEMORY;
}
- memcpy((void *)element->entry.key.str, key->str, len);
+ memcpy(element->entry.key.str, key->str, len);
break;
}
@@ -1070,8 +1082,9 @@ int hash_delete(hash_table_t *table, hash_key_t
*key)
return error;
}
}
- if (element->entry.key.type == HASH_KEY_STRING) {
- hfree(table, (char *)element->entry.key.str);
+ if (element->entry.key.type == HASH_KEY_STRING
+ || element->entry.key.type == HASH_KEY_CONST_STRING) {
+ hfree(table, element->entry.key.str);
}
hfree(table, element);
return HASH_SUCCESS;
diff --git a/dhash/dhash.h b/dhash/dhash.h
index baa0d6a..c36ab79 100644
--- a/dhash/dhash.h
+++ b/dhash/dhash.h
@@ -112,7 +112,8 @@ typedef struct hash_table_str hash_table_t;
typedef enum {
HASH_KEY_STRING,
- HASH_KEY_ULONG
+ HASH_KEY_ULONG,
+ HASH_KEY_CONST_STRING
} hash_key_enum;
typedef enum
@@ -137,6 +138,7 @@ typedef struct hash_key_t {
hash_key_enum type;
union {
char *str;
+ const char *c_str;
unsigned long ul;
};
} hash_key_t;
diff --git a/dhash/examples/dhash_test.c b/dhash/examples/dhash_test.c
index 303e9f8..7613e23 100644
--- a/dhash/examples/dhash_test.c
+++ b/dhash/examples/dhash_test.c
@@ -64,6 +64,9 @@ static char *key_string(hash_key_t *key)
case HASH_KEY_STRING:
snprintf(buf, sizeof(buf), "key string = \"%s\"", key->str);
break;
+ case HASH_KEY_CONST_STRING:
+ snprintf(buf, sizeof(buf), "key string = \"%s\"", key->c_str);
+ break;
default:
snprintf(buf, sizeof(buf), "unknown key type = %d", key->type);
break;
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>From 14618f9d49d784283894e228382a2a608d1a649e Mon Sep 17 00:00:00 2001
From: Philip Prindeville <phil...@fedoraproject.org>
Date: Mon, 3 Oct 2016 15:19:58 +0200
Subject: [PATCH 1/2] DHASH: Add new key type HASH_KEY_CONST_STRING
Add constant string as new key type. This key type
is alternative to string key type and avoids unnecessary
casts that may look dangerous (discarding const).
---
dhash/dhash.c | 29 +++++++++++++++++++++--------
dhash/dhash.h | 4 +++-
dhash/examples/dhash_test.c | 3 +++
3 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/dhash/dhash.c b/dhash/dhash.c
index 45ee0cf..98439e8 100644
--- a/dhash/dhash.c
+++ b/dhash/dhash.c
@@ -176,7 +176,7 @@ static void sys_free_wrapper(void *ptr, void *pvt)
static address_t convert_key(hash_key_t *key)
{
address_t h;
- unsigned char *k;
+ const unsigned char *k;
switch(key->type) {
case HASH_KEY_ULONG:
@@ -184,7 +184,12 @@ static address_t convert_key(hash_key_t *key)
break;
case HASH_KEY_STRING:
/* Convert string to integer */
- for (h = 0, k = (unsigned char *) key->str; *k; k++)
+ for (h = 0, k = (const unsigned char *) key->str; *k; k++)
+ h = h * PRIME_1 ^ (*k - ' ');
+ break;
+ case HASH_KEY_CONST_STRING:
+ /* Convert string to integer */
+ for (h = 0, k = (const unsigned char *) key->c_str; *k; k++)
h = h * PRIME_1 ^ (*k - ' ');
break;
default:
@@ -212,6 +217,7 @@ static bool is_valid_key_type(hash_key_enum key_type)
switch(key_type) {
case HASH_KEY_ULONG:
case HASH_KEY_STRING:
+ case HASH_KEY_CONST_STRING:
return true;
default:
return false;
@@ -244,6 +250,8 @@ static bool key_equal(hash_key_t *a, hash_key_t *b)
return (a->ul == b->ul);
case HASH_KEY_STRING:
return (strcmp(a->str, b->str) == 0);
+ case HASH_KEY_CONST_STRING:
+ return (strcmp(a->c_str, b->c_str) == 0);
}
return false;
}
@@ -670,8 +678,11 @@ int hash_destroy(hash_table_t *table)
while (p != NULL) {
q = p->next;
hdelete_callback(table, HASH_TABLE_DESTROY, &p->entry);
- if (p->entry.key.type == HASH_KEY_STRING) {
- hfree(table, (char *)p->entry.key.str);
+ if (p->entry.key.type == HASH_KEY_STRING
+ || p->entry.key.type == HASH_KEY_CONST_STRING) {
+ /* Internally we do not use constant memory for keys
+ * in hash table elements. */
+ hfree(table, p->entry.key.str);
}
hfree(table, (char *)p);
p = q;
@@ -972,13 +983,14 @@ int hash_enter(hash_table_t *table, hash_key_t *key, hash_value_t *value)
element->entry.key.ul = key->ul;
break;
case HASH_KEY_STRING:
- len = strlen(key->str)+1;
+ case HASH_KEY_CONST_STRING:
+ len = strlen(key->c_str) + 1;
element->entry.key.str = halloc(table, len);
if (element->entry.key.str == NULL) {
hfree(table, element);
return HASH_ERROR_NO_MEMORY;
}
- memcpy((void *)element->entry.key.str, key->str, len);
+ memcpy(element->entry.key.str, key->str, len);
break;
}
@@ -1070,8 +1082,9 @@ int hash_delete(hash_table_t *table, hash_key_t *key)
return error;
}
}
- if (element->entry.key.type == HASH_KEY_STRING) {
- hfree(table, (char *)element->entry.key.str);
+ if (element->entry.key.type == HASH_KEY_STRING
+ || element->entry.key.type == HASH_KEY_CONST_STRING) {
+ hfree(table, element->entry.key.str);
}
hfree(table, element);
return HASH_SUCCESS;
diff --git a/dhash/dhash.h b/dhash/dhash.h
index baa0d6a..c36ab79 100644
--- a/dhash/dhash.h
+++ b/dhash/dhash.h
@@ -112,7 +112,8 @@ typedef struct hash_table_str hash_table_t;
typedef enum {
HASH_KEY_STRING,
- HASH_KEY_ULONG
+ HASH_KEY_ULONG,
+ HASH_KEY_CONST_STRING
} hash_key_enum;
typedef enum
@@ -137,6 +138,7 @@ typedef struct hash_key_t {
hash_key_enum type;
union {
char *str;
+ const char *c_str;
unsigned long ul;
};
} hash_key_t;
diff --git a/dhash/examples/dhash_test.c b/dhash/examples/dhash_test.c
index 303e9f8..7613e23 100644
--- a/dhash/examples/dhash_test.c
+++ b/dhash/examples/dhash_test.c
@@ -64,6 +64,9 @@ static char *key_string(hash_key_t *key)
case HASH_KEY_STRING:
snprintf(buf, sizeof(buf), "key string = \"%s\"", key->str);
break;
+ case HASH_KEY_CONST_STRING:
+ snprintf(buf, sizeof(buf), "key string = \"%s\"", key->c_str);
+ break;
default:
snprintf(buf, sizeof(buf), "unknown key type = %d", key->type);
break;
--
1.8.3.1
>From d3f059cad29fcc2011cd1714cdef678ca9c9be94 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Wed, 5 Oct 2016 13:10:35 +0200
Subject: [PATCH 2/2] DHASH: Add check based unit test
---
Makefile.am | 14 ++++
dhash/dhash_ut_check.c | 178 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 192 insertions(+)
create mode 100644 dhash/dhash_ut_check.c
diff --git a/Makefile.am b/Makefile.am
index 65528a8..ca9710e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -114,12 +114,26 @@ libdhash_la_LDFLAGS = \
check_PROGRAMS += dhash_test dhash_example
TESTS += dhash_test dhash_example
+if HAVE_CHECK
+ check_PROGRAMS += dhash_ut_check
+ TESTS += dhash_ut_check
+endif
+
+
dhash_test_SOURCES = dhash/examples/dhash_test.c
dhash_test_LDADD = libdhash.la
dhash_example_SOURCES = dhash/examples/dhash_example.c
dhash_example_LDADD = libdhash.la
+dhash_ut_check_SOURCES = dhash/dhash_ut_check.c
+dhash_ut_chech_CFLAGS = $(AM_CFLAGS) \
+ $(CHECK_CFLAGS) \
+ $(NULL)
+dhash_ut_check_LDADD = libdhash.la \
+ $(CHECK_LIBS) \
+ $(NULL)
+
dist_examples_DATA += \
dhash/examples/dhash_test.c \
dhash/examples/dhash_example.c
diff --git a/dhash/dhash_ut_check.c b/dhash/dhash_ut_check.c
new file mode 100644
index 0000000..e5c8dd0
--- /dev/null
+++ b/dhash/dhash_ut_check.c
@@ -0,0 +1,178 @@
+/*
+ Authors:
+ Michal Zidek <mzi...@redhat.com>
+
+ Copyright (C) 2016 Red Hat
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "config.h"
+
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <check.h>
+
+/* #define TRACE_LEVEL 7 */
+#define TRACE_HOME
+#include "dhash.h"
+#include "path_utils.h"
+
+#define HTABLE_SIZE 128
+
+int verbose = 0;
+
+/* There must be no warnings generated during this test
+ * without having to cast the key value. */
+START_TEST(test_key_const_string)
+{
+ hash_table_t *htable;
+ int ret;
+ hash_value_t ret_val;
+ hash_value_t enter_val1 = {.type = HASH_VALUE_INT, .i = 1};
+ hash_value_t enter_val2 = {.type = HASH_VALUE_INT, .i = 2};
+ hash_key_t key = {.type = HASH_KEY_CONST_STRING, .c_str = "constant"};
+
+ ret = hash_create(HTABLE_SIZE, &htable, NULL, NULL);
+ fail_unless(ret == 0);
+
+ /* The table is empty, lookup should return error */
+ ret = hash_lookup(htable, &key, &ret_val);
+ fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
+
+ ret = hash_enter(htable, &key, &enter_val1);
+ fail_unless(ret == 0);
+
+ hash_lookup(htable, &key, &ret_val);
+ fail_unless(ret == 0);
+ fail_unless(ret_val.i == 1);
+
+ /* Overwrite the entry */
+ ret = hash_enter(htable, &key, &enter_val2);
+ fail_unless(ret == 0);
+
+ hash_lookup(htable, &key, &ret_val);
+ fail_unless(ret == 0);
+ fail_unless(ret_val.i == 2);
+
+ ret = hash_destroy(htable);
+ fail_unless(ret == 0);
+}
+END_TEST
+
+START_TEST(test_key_string)
+{
+ hash_table_t *htable;
+ int ret;
+ hash_value_t ret_val;
+ hash_value_t enter_val1 = {.type = HASH_VALUE_INT, .i = 1};
+ hash_value_t enter_val2 = {.type = HASH_VALUE_INT, .i = 2};
+ hash_key_t key;
+ char str[] = "non_constant";
+
+ key.type = HASH_KEY_STRING;
+ key.str = str;
+
+ ret = hash_create(HTABLE_SIZE, &htable, NULL, NULL);
+ fail_unless(ret == 0);
+
+ /* The table is empty, lookup should return error */
+ ret = hash_lookup(htable, &key, &ret_val);
+ fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
+
+ ret = hash_enter(htable, &key, &enter_val1);
+ fail_unless(ret == 0);
+
+ hash_lookup(htable, &key, &ret_val);
+ fail_unless(ret == 0);
+ fail_unless(ret_val.i == 1);
+
+ /* Overwrite the entry */
+ ret = hash_enter(htable, &key, &enter_val2);
+ fail_unless(ret == 0);
+
+ hash_lookup(htable, &key, &ret_val);
+ fail_unless(ret == 0);
+ fail_unless(ret_val.i == 2);
+
+ ret = hash_destroy(htable);
+ fail_unless(ret == 0);
+}
+END_TEST
+
+START_TEST(test_key_ulong)
+{
+ hash_table_t *htable;
+ int ret;
+ hash_value_t ret_val;
+ hash_value_t enter_val1 = {.type = HASH_VALUE_INT, .i = 1};
+ hash_value_t enter_val2 = {.type = HASH_VALUE_INT, .i = 2};
+ hash_key_t key = {.type = HASH_KEY_ULONG, .ul = 68ul};
+
+ ret = hash_create(HTABLE_SIZE, &htable, NULL, NULL);
+ fail_unless(ret == 0);
+
+ /* The table is empty, lookup should return error */
+ ret = hash_lookup(htable, &key, &ret_val);
+ fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
+
+ ret = hash_enter(htable, &key, &enter_val1);
+ fail_unless(ret == 0);
+
+ hash_lookup(htable, &key, &ret_val);
+ fail_unless(ret == 0);
+ fail_unless(ret_val.i == 1);
+
+ /* Overwrite the entry */
+ ret = hash_enter(htable, &key, &enter_val2);
+ fail_unless(ret == 0);
+
+ hash_lookup(htable, &key, &ret_val);
+ fail_unless(ret == 0);
+ fail_unless(ret_val.i == 2);
+
+ ret = hash_destroy(htable);
+ fail_unless(ret == 0);
+}
+END_TEST
+
+
+
+static Suite *dhash_suite(void)
+{
+ Suite *s = suite_create("");
+
+ TCase *tc_basic = tcase_create("dhash API tests");
+ tcase_add_test(tc_basic, test_key_const_string);
+ tcase_add_test(tc_basic, test_key_string);
+ tcase_add_test(tc_basic, test_key_ulong);
+ suite_add_tcase(s, tc_basic);
+
+ return s;
+}
+
+int main(void)
+{
+ int number_failed;
+
+ Suite *s = dhash_suite();
+ SRunner *sr = srunner_create(s);
+ /* If CK_VERBOSITY is set, use that, otherwise it defaults to CK_NORMAL */
+ srunner_run_all(sr, CK_ENV);
+ number_failed = srunner_ntests_failed(sr);
+ srunner_free(sr);
+ return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
--
1.8.3.1
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
