Re: [PATCH ding-libs] Extend API to const key for clients that don't need to modify their keys
by 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 <philipp(a)fedoraproject.org>
>
> 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 <stdio.h>
> #include <string.h>
> #include <stdlib.h>
> +#include <stdint.h>
> #include <errno.h>
> #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, &p->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;
> + 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;
>
7 years, 6 months
[RFC] Use GNULIB's compiler warning code
by Fabiano Fidêncio
Howdy!
I've suggested, a long time ago, that we could start making use of
GNULIB's compiler warnings from 'manywarnings' module. This is
basically what we have been doing in a few projects that I (used to
and still) maintain (like spice-gtk and libosinfo, for instance).
For now I didn't try to fix any of the warnings that we cannot cope
with, mainly because I'm not sure whether you guys will agree on using
it or not.
Here is an experimental patch that works properly on Fedora 24. I
still have to make some tests on RHEL-6, RHEL-7 and a few other
systems (Debian, at least) in order to make sure that we won't break
the build because of the patch.
If you are okay with the change, I'll start going through the warnings
that we cannot cope with and slowly start fixing them. Although, I
have the feeling that fixing some of them would cause a lot of
undesired changes, which will just bring troubles for ourselves when
backporting fixes downstream (and here I'm talking about
-Wformat-signedess, -Wsign-compare, -Wunused-parameter, ... for
instance).
I'm looking forward to hear some feedback!
Best Regards,
--
Fabiano Fidêncio
7 years, 6 months
[PATCH] BUILD: intgcheck need to fail if pytest fails
by Lukas Slebodnik
ehlo,
make intgcheck silently passed in case of failure/error in pytest.
This regression was introduced by spliting make intgcheck into
sub-targets. I am soory I had to catch that as part of review.
Simple patch is attached.
LS
7 years, 6 months
[PATCH] SPEC: Rename python packages using macro %python_provide
by Lukas Slebodnik
ehlo,
SSSD python packages were renamed in fedora few months ago.
python-* -> python2-*
But we didn't rename packages in upstream spec file and therefore
upgrade from fedora 24 -> sssd master is not possible.
Attached patch shoudl fix the issue.
BTW here are provides and obsoletes for current master
sh$ rpm -qp --provides python-libipa_hbac-1.13.92-0.fc24.x86_64.rpm
libipa_hbac-python = 1.13.92-0.fc24
python-libipa_hbac = 1.13.92-0.fc24
python-libipa_hbac(x86-64) = 1.13.92-0.fc24
sh$ rpm -qp --obsoletes python-libipa_hbac-1.13.92-0.fc24.x86_64.rpm
libipa_hbac-python < 1.12.90
and after renaming
sh$ rpm -qp --provides python2-libipa_hbac-1.13.92-0.el6.x86_64.rpm
libipa_hbac-python = 1.13.92-0.el6
python-libipa_hbac = 1.13.92-0.el6
python2-libipa_hbac = 1.13.92-0.el6
python2-libipa_hbac(x86-64) = 1.13.92-0.el6
sh$ rpm -qp --obsoletes python2-libipa_hbac-1.13.92-0.el6.x86_64.rpm
libipa_hbac-python < 1.12.90
python-libipa_hbac < 1.13.92-0.el6
LS
7 years, 6 months
Question: LDAP provider doesn't show group member
by Petr Cech
Hi,
I am working on [1] LDAP provider doesn't show group member
There is one point what I am not able to understand clearly.
I prepared environment by:
ipa user-add --first=Test --last=User1 --email=u1(a)domain.sssd cmt_user_1
ipa group-add cmt_group_1
ipa group-add-member --users=cmt_user_1 cmt_group_1
And sssd.conf is attached. I used patch 0007 [2] to simpler grep of
interesting messages. And I applied patches 0001 to 0006 (I am waiting
for their push to master).
[1] https://fedorahosted.org/sssd/ticket/3186
[2] 0007-WIP-Debuging-of-attr-member-in-ldap-answer.patch
What I saw is:
# COMMAND ON CLIENT:
[root@mirach sssd]# systemctl daemon-reload && sudo su -c "truncate -s0
/var/log/sssd/*.log" && sudo su -c "rm -f /var/lib/sss/db/*" && sudo su
-c "rm -f /var/lib/sss/mc/*" && sudo systemctl restart sssd.service &&
getent group cmt_group_1 && grep '>>>' *.log
# There is user missing:
cmt_group_1:*:1703800166:
# SSSD LOG:
[sdap_get_generic_ext_step] (0x0400): >>> calling ldap_search_ext with
[(&(cn=cmt_group_1)(objectClass=posixGroup)(cn=*)(&(gidNumber=*)(!(gidNumber=0))))][cn=groups,cn=accounts,dc=beta].
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [objectClass]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [cn]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [userPassword]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [gidNumber]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [member]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs:
[modifyTimestamp]
[sdap_get_generic_ext_step] (0x1000): >>> Requesting attrs: [entryUSN]
[sdap_parse_entry] (0x0080): >>> ----- filter map:
[sdap_parse_entry] (0x0080): >>> [posixGroup]
[sdap_parse_entry] (0x0080): >>> [(null)]
[sdap_parse_entry] (0x0080): >>> [cn]
[sdap_parse_entry] (0x0080): >>> [userPassword]
[sdap_parse_entry] (0x0080): >>> [gidNumber]
[sdap_parse_entry] (0x0080): >>> [member]
[sdap_parse_entry] (0x0080): >>> [(null)]
[sdap_parse_entry] (0x0080): >>> [(null)]
[sdap_parse_entry] (0x0080): >>> [modifyTimestamp]
[sdap_parse_entry] (0x0080): >>> [entryUSN]
[sdap_parse_entry] (0x0080): >>> [(null)]
[sdap_parse_entry] (0x0080): >>> [(null)]
[sdap_parse_entry] (0x0080): >>> ----- attributes in ldap answer:
[sdap_parse_entry] (0x0020): >>> [objectClass]
[sdap_parse_entry] (0x0020): >>> [cn]
[sdap_parse_entry] (0x0020): >>> [gidNumber]
[sdap_parse_entry] (0x0020): >>> [modifyTimestamp]
[sdap_parse_entry] (0x0020): >>> [entryUSN]
[sdap_get_groups_process] (0x0400): >>> Group contains:
[sdap_get_groups_process] (0x0400): >>> 0: [originalDN]
[sdap_get_groups_process] (0x0400): >>> 1: [name]
[sdap_get_groups_process] (0x0400): >>> 2: [gidNumber]
[sdap_get_groups_process] (0x0400): >>> 3: [originalModifyTimestamp]
[sdap_get_groups_process] (0x0400): >>> 4: [entryUSN]
[sdap_nested_group_process_send] (0x2000): >>> About to process group
[cn=cmt_group_1,cn=groups,cn=accounts,dc=beta]
# LDAPSEARCH ON LDAP SERVER:
[pcech@algol ~]$ ldapsearch -H ldap://localhost -D
uid=admin,cn=users,cn=accounts,dc=beta -w myspulin -b
cn=groups,cn=accounts,dc=beta
"(&(cn=cmt_group_1)(objectClass=posixGroup)(cn=*)(&(gidNumber=*)(!(gidNumber=0))))"
# extended LDIF
#
# LDAPv3
# base <cn=groups,cn=accounts,dc=beta> with scope subtree
# filter:
(&(cn=cmt_group_1)(objectClass=posixGroup)(cn=*)(&(gidNumber=*)(!(gidNumber=0))))
# requesting: ALL
#
# cmt_group_1, groups, accounts, beta
dn: cn=cmt_group_1,cn=groups,cn=accounts,dc=beta
objectClass: ipaobject
objectClass: top
objectClass: ipausergroup
objectClass: posixgroup
objectClass: groupofnames
objectClass: nestedgroup
cn: cmt_group_1
ipaUniqueID: db0b564e-7efe-11e6-a757-5254001a3efa
gidNumber: 1703800166
member: uid=cmt_user_1,cn=users,cn=accounts,dc=beta
# search result
search: 2
result: 0 Success
# numResponses: 2
# numEntries: 1
Question is why didn't SSSD obtain member attribute if I saw it in
ldapsearch call? Is there something what I miss?
Lukas pointed out that it may not be the correct way to use IPA for
handling users in groups in LDAP server. I understand I am working on AD
case too.
But -- is it really possible to get different data via SSSD and
ldapsearch() with the same filter and base?
Regards
--
Petr^4 Čech
7 years, 6 months
