[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Philip Prindeville 

> On Oct 5, 2016, at 9:43 AM, Jakub Hrozek  wrote:
> 
> On Wed, Oct 05, 2016 at 07:42:23AM -0600, Philip Prindeville wrote:
>> On Oct 5, 2016, at 5:45 AM, 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.
>>> 
>> 
>> 
>> The project was an SSSD provider (proxy) for Tacacs+ accounting, 
>> authentication, and authorization.
> 
> Wow, I'm intrigued and I think all of us want to help!
> 
> Is that an in-house project or do you plan on open sourcing it? If the
> latter, I wonder if we could help with review, writing more docs or just
> about anything?

Sure, we were planning on upstreaming it.

For the Tacacs+ part, I had used an event-driven dispatcher (libevent), but the 
DBus listener ran in a separate thread…

I had wanted to make everything be event-driven, including servicing the 
arrival of DBus requests from SSSD, because then you could handle request/reply 
pairs asynchronously for Tacacs, which then enables you to let Tacacs+ reuse 
connections to servers if they’re already up, or do multiplexing (i.e. having 
multiple requests on the fly, and having responses come back out of order, etc).

That part I haven’t figured out yet… how to integrate DBus with libevent.  
There are a couple of posting online about how to do this, but nothing that 
looks very elegant.  See:

https://lists.freedesktop.org/archives/dbus/2016-August/017005.html 


The one example I saw 
(http://stackoverflow.com/questions/9378593/dbuswatch-and-dbustimeout-examples) 
struck me as having an unfortunate race condition.  I looked at that and 
thought, “there must be a better way”.

So once I clear that hurdle, and the changes I had to make to pam_tacplus’s 
libtac library get accepted upstream (I’m about a third of the way done) then I 
can finish the rewrite (sans the ugly libevent/pthread mix that it currently 
has) and upstream the entire thing.

I’ve not looking into the GDBus interface, just the old glib-dbus.

Any advice on how to handle that particular problem would be especially helpful.

Thanks,

-Philip


___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Michal Židek 

On 10/05/2016 04:48 PM, Petr Cech wrote:



On 10/05/2016 04:39 PM, Michal Židek wrote:

On 10/05/2016 04:30 PM, Petr Cech wrote:

On 10/05/2016 04:18 PM, Michal Židek wrote:

On 10/05/2016 03:47 PM, Philip Prindeville wrote:



On Oct 5, 2016, at 7:18 AM, Michal Židek  wrote:



Hello Michal,

I comment two things online.


I agree with the comments. New test attached.



0002-DHASH-Add-check-based-unit-test.patch


From 83b18c3ca4d70086bbdc645e0d09e7e027e5e9b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Wed, 5 Oct 2016 13:10:35 +0200
Subject: [PATCH 2/2] DHASH: Add check based unit test

---


LGTM.
ACK.

Regards



Pushed to ding-libs master:
* d713c1f979ba3e046a6dd53f5262ae0cdbe53bc4
* f085b9b26f6e00cec4a0acf6896843436234ca4f

Michal
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Michal Židek 

On 10/05/2016 05:43 PM, Jakub Hrozek wrote:

On Wed, Oct 05, 2016 at 07:42:23AM -0600, Philip Prindeville wrote:

On Oct 5, 2016, at 5:45 AM, 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.




The project was an SSSD provider (proxy) for Tacacs+ accounting, 
authentication, and authorization.


Wow, I'm intrigued and I think all of us want to help!

Is that an in-house project or do you plan on open sourcing it? If the
latter, I wonder if we could help with review, writing more docs or just
about anything?



I agree that this looks interesting. It would be good if
the discussion about it took place here on the sssd devel-list,
and maybe we could give a hand with something.

Michal
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Jakub Hrozek 
On Wed, Oct 05, 2016 at 07:42:23AM -0600, Philip Prindeville wrote:
> On Oct 5, 2016, at 5:45 AM, 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.
> > 
> 
> 
> The project was an SSSD provider (proxy) for Tacacs+ accounting, 
> authentication, and authorization.

Wow, I'm intrigued and I think all of us want to help!

Is that an in-house project or do you plan on open sourcing it? If the
latter, I wonder if we could help with review, writing more docs or just
about anything?
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Petr Cech 



On 10/05/2016 04:39 PM, Michal Židek wrote:

On 10/05/2016 04:30 PM, Petr Cech wrote:

On 10/05/2016 04:18 PM, Michal Židek wrote:

On 10/05/2016 03:47 PM, Philip Prindeville wrote:



On Oct 5, 2016, at 7:18 AM, Michal Židek  wrote:



Hello Michal,

I comment two things online.


I agree with the comments. New test attached.



0002-DHASH-Add-check-based-unit-test.patch


From 83b18c3ca4d70086bbdc645e0d09e7e027e5e9b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Wed, 5 Oct 2016 13:10:35 +0200
Subject: [PATCH 2/2] DHASH: Add check based unit test

---


LGTM.
ACK.

Regards

--
Petr^4 Čech
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Michal Židek 

On 10/05/2016 04:30 PM, Petr Cech wrote:

On 10/05/2016 04:18 PM, Michal Židek wrote:

On 10/05/2016 03:47 PM, Philip Prindeville wrote:



On Oct 5, 2016, at 7:18 AM, Michal Židek  wrote:

I forgot to attach the patches.

Again the first one is acked by me, the second
needs a review.

Michal



Thanks for writing those tests.

Minor comment, dhash_ut_check.c and the existing checks don’t have any
negative tests, such as attempting to delete a non-existent key, or
deleting an already deleted key…  or entering a key which is already
present.

-Philip



Good point. I added some delete operations to these tests.
However these are just some sanity tests to cover the code that
was changed. My intention was not to test everything here.

New tests are attached.

Michal


Hello Michal,

I comment two things online.


I agree with the comments. New test attached.






0002-DHASH-Add-check-based-unit-test.patch


From 83b18c3ca4d70086bbdc645e0d09e7e027e5e9b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
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 | 210
+
 2 files changed, 224 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 000..b4b36fa
--- /dev/null
+++ b/dhash/dhash_ut_check.c
@@ -0,0 +1,210 @@
+/*
+Authors:
+Michal Zidek 
+
+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
.
+*/
+
+#include "config.h"
+
+#include 
+#include 
+#include 

IMO, this is unnecessary.



+#include 
+#include 
+
+/* #define TRACE_LEVEL 7 */
+#define TRACE_HOME
+#include "dhash.h"
+#include "path_utils.h"

IMO, this is unnecessary.


+
+#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, , NULL, NULL);
+fail_unless(ret == 0);
+
+/* The table is empty, lookup should return error */
+ret = hash_lookup(htable, , _val);
+fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
+
+/* Deleting with non-existing key should return error */
+ret = hash_delete(htable, );
+fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
+
+ret = hash_enter(htable, , _val1);
+fail_unless(ret == 0);
+
+hash_lookup(htable, , _val);
+fail_unless(ret == 0);
+fail_unless(ret_val.i == 1);
+
+/* Overwrite the entry */
+ret = hash_enter(htable, , _val2);
+fail_unless(ret == 0);
+
+hash_lookup(htable, , _val);
+fail_unless(ret == 0);
+fail_unless(ret_val.i == 2);
+
+ret = hash_delete(htable, );
+fail_unless(ret == 0);
+
+/* Delete again with the same key */
+ret = hash_delete(htable, );
+fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
+
+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;

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Petr Cech 

On 10/05/2016 04:18 PM, Michal Židek wrote:

On 10/05/2016 03:47 PM, Philip Prindeville wrote:



On Oct 5, 2016, at 7:18 AM, Michal Židek  wrote:

I forgot to attach the patches.

Again the first one is acked by me, the second
needs a review.

Michal



Thanks for writing those tests.

Minor comment, dhash_ut_check.c and the existing checks don’t have any
negative tests, such as attempting to delete a non-existent key, or
deleting an already deleted key…  or entering a key which is already
present.

-Philip



Good point. I added some delete operations to these tests.
However these are just some sanity tests to cover the code that
was changed. My intention was not to test everything here.

New tests are attached.

Michal


Hello Michal,

I comment two things online.




0002-DHASH-Add-check-based-unit-test.patch


From 83b18c3ca4d70086bbdc645e0d09e7e027e5e9b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
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 | 210 +
 2 files changed, 224 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 000..b4b36fa
--- /dev/null
+++ b/dhash/dhash_ut_check.c
@@ -0,0 +1,210 @@
+/*
+Authors:
+Michal Zidek 
+
+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 .
+*/
+
+#include "config.h"
+
+#include 
+#include 
+#include 

IMO, this is unnecessary.



+#include 
+#include 
+
+/* #define TRACE_LEVEL 7 */
+#define TRACE_HOME
+#include "dhash.h"
+#include "path_utils.h"

IMO, this is unnecessary.


+
+#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, , NULL, NULL);
+fail_unless(ret == 0);
+
+/* The table is empty, lookup should return error */
+ret = hash_lookup(htable, , _val);
+fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
+
+/* Deleting with non-existing key should return error */
+ret = hash_delete(htable, );
+fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
+
+ret = hash_enter(htable, , _val1);
+fail_unless(ret == 0);
+
+hash_lookup(htable, , _val);
+fail_unless(ret == 0);
+fail_unless(ret_val.i == 1);
+
+/* Overwrite the entry */
+ret = hash_enter(htable, , _val2);
+fail_unless(ret == 0);
+
+hash_lookup(htable, , _val);
+fail_unless(ret == 0);
+fail_unless(ret_val.i == 2);
+
+ret = hash_delete(htable, );
+fail_unless(ret == 0);
+
+/* Delete again with the same key */
+ret = hash_delete(htable, );
+fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
+
+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

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Michal Židek 

On 10/05/2016 03:47 PM, Philip Prindeville wrote:



On Oct 5, 2016, at 7:18 AM, Michal Židek  wrote:

I forgot to attach the patches.

Again the first one is acked by me, the second
needs a review.

Michal



Thanks for writing those tests.

Minor comment, dhash_ut_check.c and the existing checks don’t have any negative 
tests, such as attempting to delete a non-existent key, or deleting an already 
deleted key…  or entering a key which is already present.

-Philip



Good point. I added some delete operations to these tests.
However these are just some sanity tests to cover the code that
was changed. My intention was not to test everything here.

New tests are attached.

Michal
>From 14618f9d49d784283894e228382a2a608d1a649e Mon Sep 17 00:00:00 2001
From: Philip Prindeville 
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, >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 {

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Philip Prindeville 

> On Oct 5, 2016, at 7:18 AM, Michal Židek  wrote:
> 
> I forgot to attach the patches.
> 
> Again the first one is acked by me, the second
> needs a review.
> 
> Michal


Thanks for writing those tests.

Minor comment, dhash_ut_check.c and the existing checks don’t have any negative 
tests, such as attempting to delete a non-existent key, or deleting an already 
deleted key…  or entering a key which is already present.

-Philip


___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Philip Prindeville 
On Oct 5, 2016, at 5:45 AM, 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.
> 


The project was an SSSD provider (proxy) for Tacacs+ accounting, 
authentication, and authorization.

-Philip
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Michal Židek 

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 

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, >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

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-10-05 Thread Michal Židek 

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 

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, 
>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

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-09-30 Thread Michal Židek 

Hello Philip,

please read the comments inline. I also
attached git diff to demonstrate what I mean
to this email.


 From: Philip Prindeville 


Add c_str as a "const char *" to the hash_key_t.

---
  dhash/dhash.c   | 35 ++-
  dhash/dhash.h   |  4 +++-
  dhash/examples/dhash_test.c |  3 +++
  3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/dhash/dhash.c b/dhash/dhash.c
index 45ee0cf..5f9f631 100644
--- a/dhash/dhash.c
+++ b/dhash/dhash.c
@@ -44,6 +44,7 @@
  #include 
  #include 
  #include 
+#include 


Why did you include stdint?


  #include 
  #include "dhash.h"

@@ -70,6 +71,9 @@
  } \
  } while(0)

+#define discard_const(ptr) ((void *)((intptr_t)(ptr)))
+#define discard_const_p(type, ptr) ((type *)discard_const(ptr))
+


I do not think it is good to use the discard_const macro for
this case. See further comments below.



/*/

  /** Internal Type Definitions
/

/*/

@@ -176,7 +180,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 +188,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 +221,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 +254,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;
  }
@@ -671,7 +683,9 @@ int hash_destroy(hash_table_t *table)
  q = p->next;
  hdelete_callback(table,
HASH_TABLE_DESTROY, >entry);
  if (p->entry.key.type ==
HASH_KEY_STRING) {
-hfree(table, (char
*)p->entry.key.str);
+hfree(table, p->entry.key.str);
+} else if (p->entry.key.type ==
HASH_KEY_CONST_STRING) {
+hfree(table,
discard_const(p->entry.key.c_str));
  }
  hfree(table, (char *)p);
  p = q;
@@ -978,7 +992,16 @@ int hash_enter(hash_table_t *table, hash_key_t
*key, hash_value_t *value)
  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;
+case HASH_KEY_CONST_STRING:
+len = strlen(key->c_str)+1;
+element->entry.key.c_str = halloc(table, len);
+if (element->entry.key.c_str == NULL) {
+hfree(table, element);
+return HASH_ERROR_NO_MEMORY;
+}
+memcpy(discard_const(element->entry.key.c_str),
key->c_str, len);
  break;


We do an internal copy here.
We do not need to special case the HASH_KEY_CONST_STRING.
Internally we can always access the key in the hash table
with key.str and the key from the user with key.c_str
no matter if the user chose constant or non constant key.
We will only modify the internal part during free operation
and do not touch the key from user.


  }

@@ -1071,7 +1094,9 @@ int hash_delete(hash_table_t *table,
hash_key_t *key)
  }
  }
  if (element->entry.key.type == HASH_KEY_STRING) {
-hfree(table, (char *)element->entry.key.str);
+hfree(table, element->entry.key.str);
+} else if (element->entry.key.type == HASH_KEY_CONST_STRING) {
+hfree(table, discard_const(element->entry.key.c_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

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-09-30 Thread Michal Židek 

Hi!

Ding-libs originates in SSSD project and shares
a lot of SSSD infrastructure. Mailing
list for ding-libs and SSSD is the same,

You can subscribe to the mailing list here:
https://lists.fedorahosted.org/admin/lists/sssd-devel.lists.fedorahosted.org/

Ding-libs and SSSD also share the ticketing
system and wiki (trac):
https://fedorahosted.org/sssd/

In order to subscribe to the mailing list
or login to the trac instance, you will need
a FAS account. This account is used to
login to most of fedorahosted services
(including for example copr).

Michal

On 09/29/2016 07:43 PM, Philip Prindeville wrote:

Is there a ding-libs website?

I wanted to subscribe to the mailing list and read the archives but 
fedorahosted.org only seems to point to the git repo… nothing about a ding-libs 
general website, etc.



On Sep 29, 2016, at 4:50 AM, Michal Židek  wrote:

Moving this to sssd-devel list. So that other
developers can see the patch and review process.

I will start the review after today's meeting.

Michal

(Lukas, Stephen, I put you two to CC only because you
were also original recipients of the mail, I will not
CC you further)

On 09/28/2016 03:00 AM, Philip Prindeville wrote:

From: Philip Prindeville 

Add c_str as a "const char *" to the hash_key_t.

---
  dhash/dhash.c   | 35 ++-
  dhash/dhash.h   |  4 +++-
  dhash/examples/dhash_test.c |  3 +++
  3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/dhash/dhash.c b/dhash/dhash.c
index 45ee0cf..5f9f631 100644
--- a/dhash/dhash.c
+++ b/dhash/dhash.c
@@ -44,6 +44,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include "dhash.h"

@@ -70,6 +71,9 @@
  } \
  } while(0)

+#define discard_const(ptr) ((void *)((intptr_t)(ptr)))
+#define discard_const_p(type, ptr) ((type *)discard_const(ptr))
+
  
/*/
  /** Internal Type Definitions 
/
  
/*/
@@ -176,7 +180,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 +188,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 +221,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 +254,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;
  }
@@ -671,7 +683,9 @@ int hash_destroy(hash_table_t *table)
  q = p->next;
  hdelete_callback(table, HASH_TABLE_DESTROY, 
>entry);
  if (p->entry.key.type == HASH_KEY_STRING) {
-hfree(table, (char *)p->entry.key.str);
+hfree(table, p->entry.key.str);
+} else if (p->entry.key.type == 
HASH_KEY_CONST_STRING) {
+hfree(table, 
discard_const(p->entry.key.c_str));
  }
  hfree(table, (char *)p);
  p = q;
@@ -978,7 +992,16 @@ int hash_enter(hash_table_t *table, hash_key_t *key, 
hash_value_t *value)
  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;
+case HASH_KEY_CONST_STRING:
+len = strlen(key->c_str)+1;
+element->entry.key.c_str = halloc(table, len);
+if (element->entry.key.c_str == NULL) {
+hfree(table, element);
+return HASH_ERROR_NO_MEMORY;
+}
+memcpy(discard_const(element->entry.key.c_str), key->c_str, len);
  break;
  }

@@ -1071,7 +1094,9 @@ int

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-09-29 Thread Philip Prindeville 
Is there a ding-libs website?

I wanted to subscribe to the mailing list and read the archives but 
fedorahosted.org only seems to point to the git repo… nothing about a ding-libs 
general website, etc.


> On Sep 29, 2016, at 4:50 AM, Michal Židek  wrote:
> 
> Moving this to sssd-devel list. So that other
> developers can see the patch and review process.
> 
> I will start the review after today's meeting.
> 
> Michal
> 
> (Lukas, Stephen, I put you two to CC only because you
> were also original recipients of the mail, I will not
> CC you further)
> 
> On 09/28/2016 03:00 AM, Philip Prindeville wrote:
>> From: Philip Prindeville 
>> 
>> Add c_str as a "const char *" to the hash_key_t.
>> 
>> ---
>>  dhash/dhash.c   | 35 ++-
>>  dhash/dhash.h   |  4 +++-
>>  dhash/examples/dhash_test.c |  3 +++
>>  3 files changed, 36 insertions(+), 6 deletions(-)
>> 
>> diff --git a/dhash/dhash.c b/dhash/dhash.c
>> index 45ee0cf..5f9f631 100644
>> --- a/dhash/dhash.c
>> +++ b/dhash/dhash.c
>> @@ -44,6 +44,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include "dhash.h"
>> 
>> @@ -70,6 +71,9 @@
>>  } \
>>  } while(0)
>> 
>> +#define discard_const(ptr) ((void *)((intptr_t)(ptr)))
>> +#define discard_const_p(type, ptr) ((type *)discard_const(ptr))
>> +
>>  
>> /*/
>>  /** Internal Type Definitions 
>> /
>>  
>> /*/
>> @@ -176,7 +180,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 +188,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 +221,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 +254,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;
>>  }
>> @@ -671,7 +683,9 @@ int hash_destroy(hash_table_t *table)
>>  q = p->next;
>>  hdelete_callback(table, HASH_TABLE_DESTROY, 
>> >entry);
>>  if (p->entry.key.type == HASH_KEY_STRING) {
>> -hfree(table, (char *)p->entry.key.str);
>> +hfree(table, p->entry.key.str);
>> +} else if (p->entry.key.type == 
>> HASH_KEY_CONST_STRING) {
>> +hfree(table, 
>> discard_const(p->entry.key.c_str));
>>  }
>>  hfree(table, (char *)p);
>>  p = q;
>> @@ -978,7 +992,16 @@ int hash_enter(hash_table_t *table, hash_key_t *key, 
>> hash_value_t *value)
>>  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;
>> +case HASH_KEY_CONST_STRING:
>> +len = strlen(key->c_str)+1;
>> +element->entry.key.c_str = halloc(table, len);
>> +if (element->entry.key.c_str == NULL) {
>> +hfree(table, element);
>> +return HASH_ERROR_NO_MEMORY;
>> +}
>> +memcpy(discard_const(element->entry.key.c_str), key->c_str, 
>> len);
>>  break;
>>  }
>> 
>> @@ -1071,7 +1094,9 @@ int hash_delete(hash_table_t *table, hash_key_t *key)
>>  }
>>  }
>>  if (element->entry.key.type == HASH_KEY_STRING) {
>> -hfree(table, (char *)element->entry.key.str);
>> +hfree(table, element->entry.key.str);
>> +} else if (element->entry.key.type ==

[SSSD] Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys

2016-09-29 Thread Michal Židek 

Moving this to sssd-devel list. So that other
developers can see the patch and review process.

I will start the review after today's meeting.

Michal

(Lukas, Stephen, I put you two to CC only because you
were also original recipients of the mail, I will not
CC you further)

On 09/28/2016 03:00 AM, Philip Prindeville wrote:

From: Philip Prindeville 

Add c_str as a "const char *" to the hash_key_t.

---
  dhash/dhash.c   | 35 ++-
  dhash/dhash.h   |  4 +++-
  dhash/examples/dhash_test.c |  3 +++
  3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/dhash/dhash.c b/dhash/dhash.c
index 45ee0cf..5f9f631 100644
--- a/dhash/dhash.c
+++ b/dhash/dhash.c
@@ -44,6 +44,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include "dhash.h"

@@ -70,6 +71,9 @@
  } \
  } while(0)

+#define discard_const(ptr) ((void *)((intptr_t)(ptr)))
+#define discard_const_p(type, ptr) ((type *)discard_const(ptr))
+
  
/*/
  /** Internal Type Definitions 
/
  
/*/
@@ -176,7 +180,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 +188,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 +221,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 +254,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;
  }
@@ -671,7 +683,9 @@ int hash_destroy(hash_table_t *table)
  q = p->next;
  hdelete_callback(table, HASH_TABLE_DESTROY, 
>entry);
  if (p->entry.key.type == HASH_KEY_STRING) {
-hfree(table, (char *)p->entry.key.str);
+hfree(table, p->entry.key.str);
+} else if (p->entry.key.type == 
HASH_KEY_CONST_STRING) {
+hfree(table, 
discard_const(p->entry.key.c_str));
  }
  hfree(table, (char *)p);
  p = q;
@@ -978,7 +992,16 @@ int hash_enter(hash_table_t *table, hash_key_t *key, 
hash_value_t *value)
  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;
+case HASH_KEY_CONST_STRING:
+len = strlen(key->c_str)+1;
+element->entry.key.c_str = halloc(table, len);
+if (element->entry.key.c_str == NULL) {
+hfree(table, element);
+return HASH_ERROR_NO_MEMORY;
+}
+memcpy(discard_const(element->entry.key.c_str), key->c_str, len);
  break;
  }

@@ -1071,7 +1094,9 @@ int hash_delete(hash_table_t *table, hash_key_t *key)
  }
  }
  if (element->entry.key.type == HASH_KEY_STRING) {
-hfree(table, (char *)element->entry.key.str);
+hfree(table, element->entry.key.str);
+} else if (element->entry.key.type == HASH_KEY_CONST_STRING) {
+hfree(table, discard_const(element->entry.key.c_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;
+