Re: [SSSD] [PATCH] add sysdb_delete_recursive request to sysdb API

2009-10-30 Thread Sumit Bose 
On Fri, Oct 30, 2009 at 05:42:10PM -0400, Simo Sorce wrote:
> On Fri, 2009-10-30 at 12:01 +0100, Sumit Bose wrote:
> > On Thu, Oct 29, 2009 at 11:26:39PM +0100, Sumit Bose wrote:
> > > On Thu, Oct 29, 2009 at 09:32:34PM +, Simo Sorce wrote:
> > > > On Thu, 2009-10-29 at 19:40 +0100, Sumit Bose wrote:
> > > > > On Thu, Oct 29, 2009 at 01:39:21PM +0100, Sumit Bose wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > this patch adds a recursive delete request to the sysdb API. It has
> > > > > the
> > > > > > same interface as sysdb_delete_entry, but does not delete the entry,
> > > > > but
> > > > > > its children.
> > > > > > 
> > > > > > bye,
> > > > > > Sumit
> > > > > 
> > > > > This is a new version of the patch which tries to delete the entry AND
> > > > > all its children. It searches all objects with a subtree search, sorts
> > > > > the result so that the ones with the most components come first and
> > > > > finally loops over the results and deletes them.
> > > > 
> > > > Comments inline.
> > > > 
> > > > > +
> > > > > +subreq = sysdb_search_entry_send(state, ev, handle, dn,
> > > > > LDB_SCOPE_SUBTREE,
> > > > > + "distinguishedName=*", NULL);
> > > > 
> > > > Please use "(objectclass=*)" as filter to catch all entries.
> > > > 
> > > 
> > > I would prefer to stay with distinguishedName, because it is
> > > auto-generated and always present.
> > > 
> > > > Also please set attrs. Passing NULL, means you will retrieve all
> > > > attributes wasting a lot of memory unnecessarily. You are interested
> > > > only in the entries msg->dn, so you probably do not want any attribute
> > > > returned at all.
> > > 
> > > ah, I thought NULL means nothing, now I pass { NULL }
> > > 
> > > > 
> > > > [..]
> > > > 
> > > > > +static int compare_ldb_dn_comp_num(const void *m1, const void *m2)
> > > > > +{
> > > > > +struct ldb_message *msg1 = talloc_get_type(*(const void **) m1,
> > > > > +   struct ldb_message);
> > > > > +struct ldb_message *msg2 = talloc_get_type(*(const void **) m2,
> > > > > +   struct ldb_message);
> > > > > +
> > > > > +return ldb_dn_get_comp_num(msg2->dn) -
> > > > > ldb_dn_get_comp_num(msg1->dn);
> > > > > +}
> > > > 
> > > > Please move this function in sysdb.c, it's a generic function that can
> > > > be used by multiple functions and here just interrupts reading the
> > > > program flow.
> > > 
> > > done
> > > 
> > > > 
> > > > > +static void sysdb_delete_recursive_loop(struct tevent_req *subreq)
> > > > [...]
> > > > 
> > > > I think you should split the this function into a function that receives
> > > > the results of sysdb_search_entry_recv() and then another one that sets
> > > > the loop. If necessary use the trick I used in sdap_cli_connect to do
> > > > continuation functions (see the sdap_cli_*_step functions).
> > > > 
> > > 
> > > done
> > > 
> > > > The rest looks good to me.
> > > > 
> > > 
> > > Thanks for reviewing.
> > > 
> > > bye,
> > > Sumit
> > > 
> > 
> > sorry, this new patch fixes a compiler warning.
> 
> Looks good to me, I have only a minor nitpick, shouldn't the ENOENT
> error in sysdb_delete_recursive_op_done() be fatal ?
> 
> Given it should never happen, does it make sense to allow to continue ?
> 
> Simo.
> 

ok, it is now fatal as all other errors

bye,
Sumit
>From de69fe0de87ede1acabc37a94070a2f932a3ea00 Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Thu, 29 Oct 2009 12:57:57 +0100
Subject: [PATCH] add sysdb_delete_recursive request to sysdb API

---
 server/db/sysdb.c  |   12 
 server/db/sysdb.h  |   10 +++
 server/db/sysdb_ops.c  |  152 
 server/tests/sysdb-tests.c |  111 +++-
 4 files changed, 281 insertions(+), 4 deletions(-)

diff --git a/server/db/sysdb.c b/server/db/sysdb.c
index 5811ddc..a2ac3b2 100644
--- a/server/db/sysdb.c
+++ b/server/db/sysdb.c
@@ -1417,3 +1417,15 @@ int sysdb_get_ctx_from_list(struct sysdb_ctx_list 
*ctx_list,
 /* definitely not found */
 return ENOENT;
 }
+
+
+int compare_ldb_dn_comp_num(const void *m1, const void *m2)
+{
+struct ldb_message *msg1 = talloc_get_type(*(void **) discard_const(m1),
+   struct ldb_message);
+struct ldb_message *msg2 = talloc_get_type(*(void **) discard_const(m2),
+   struct ldb_message);
+
+return ldb_dn_get_comp_num(msg2->dn) - ldb_dn_get_comp_num(msg1->dn);
+}
+
diff --git a/server/db/sysdb.h b/server/db/sysdb.h
index 00a3378..72f56db 100644
--- a/server/db/sysdb.h
+++ b/server/db/sysdb.h
@@ -190,6 +190,8 @@ struct ldb_dn *sysdb_custom_dn(struct sysdb_ctx *ctx, void 
*memctx,
 struct ldb_context *sysdb_ctx_get_ldb(struct sysdb_ctx *ctx);
 struct ldb_context *sysdb_handle_get_ldb(struct sysdb_handle *handle);
 
+int compare_ldb_dn_comp_num(const void *

Re: [SSSD] [PATCH] add sysdb_delete_recursive request to sysdb API

2009-10-30 Thread Simo Sorce 
On Fri, 2009-10-30 at 12:01 +0100, Sumit Bose wrote:
> On Thu, Oct 29, 2009 at 11:26:39PM +0100, Sumit Bose wrote:
> > On Thu, Oct 29, 2009 at 09:32:34PM +, Simo Sorce wrote:
> > > On Thu, 2009-10-29 at 19:40 +0100, Sumit Bose wrote:
> > > > On Thu, Oct 29, 2009 at 01:39:21PM +0100, Sumit Bose wrote:
> > > > > Hi,
> > > > > 
> > > > > this patch adds a recursive delete request to the sysdb API. It has
> > > > the
> > > > > same interface as sysdb_delete_entry, but does not delete the entry,
> > > > but
> > > > > its children.
> > > > > 
> > > > > bye,
> > > > > Sumit
> > > > 
> > > > This is a new version of the patch which tries to delete the entry AND
> > > > all its children. It searches all objects with a subtree search, sorts
> > > > the result so that the ones with the most components come first and
> > > > finally loops over the results and deletes them.
> > > 
> > > Comments inline.
> > > 
> > > > +
> > > > +subreq = sysdb_search_entry_send(state, ev, handle, dn,
> > > > LDB_SCOPE_SUBTREE,
> > > > + "distinguishedName=*", NULL);
> > > 
> > > Please use "(objectclass=*)" as filter to catch all entries.
> > > 
> > 
> > I would prefer to stay with distinguishedName, because it is
> > auto-generated and always present.
> > 
> > > Also please set attrs. Passing NULL, means you will retrieve all
> > > attributes wasting a lot of memory unnecessarily. You are interested
> > > only in the entries msg->dn, so you probably do not want any attribute
> > > returned at all.
> > 
> > ah, I thought NULL means nothing, now I pass { NULL }
> > 
> > > 
> > > [..]
> > > 
> > > > +static int compare_ldb_dn_comp_num(const void *m1, const void *m2)
> > > > +{
> > > > +struct ldb_message *msg1 = talloc_get_type(*(const void **) m1,
> > > > +   struct ldb_message);
> > > > +struct ldb_message *msg2 = talloc_get_type(*(const void **) m2,
> > > > +   struct ldb_message);
> > > > +
> > > > +return ldb_dn_get_comp_num(msg2->dn) -
> > > > ldb_dn_get_comp_num(msg1->dn);
> > > > +}
> > > 
> > > Please move this function in sysdb.c, it's a generic function that can
> > > be used by multiple functions and here just interrupts reading the
> > > program flow.
> > 
> > done
> > 
> > > 
> > > > +static void sysdb_delete_recursive_loop(struct tevent_req *subreq)
> > > [...]
> > > 
> > > I think you should split the this function into a function that receives
> > > the results of sysdb_search_entry_recv() and then another one that sets
> > > the loop. If necessary use the trick I used in sdap_cli_connect to do
> > > continuation functions (see the sdap_cli_*_step functions).
> > > 
> > 
> > done
> > 
> > > The rest looks good to me.
> > > 
> > 
> > Thanks for reviewing.
> > 
> > bye,
> > Sumit
> > 
> 
> sorry, this new patch fixes a compiler warning.

Looks good to me, I have only a minor nitpick, shouldn't the ENOENT
error in sysdb_delete_recursive_op_done() be fatal ?

Given it should never happen, does it make sense to allow to continue ?

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] Problem or not?

2009-10-30 Thread Sumit Bose 
On Fri, Oct 30, 2009 at 01:54:19PM -0700, Jeff Schroeder wrote:
> I've built this package for Fedora 10 and am testing it out.
> http://kojipkgs.fedoraproject.org/packages/sssd/0.7.1/1.fc12/src/sssd-0.7.1-1.fc12.src.rpm
> 
> In /var/log/sssd/sssd.log:
> [sssd[be[LDAP]]] [load_backend_module] (0): Unable to load init fn
> sssm_ldap_access_init from module ldap, error:
> /usr/lib64/sssd/libsss_ldap.so: undefined symbol:
> sssm_ldap_access_init
> 
> Is this a problem? Authentication appears to be working but I'm not
> sure 100% as that can't be good.
> 

This is just a warning. There is already a bug filed to make this message less 
irritating.

bye,
Sumit

> -- 
> Jeff Schroeder
> 
> Don't drink and derive, alcohol and analysis don't mix.
> http://www.digitalprognosis.com
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

[SSSD] Problem or not?

2009-10-30 Thread Jeff Schroeder 
I've built this package for Fedora 10 and am testing it out.
http://kojipkgs.fedoraproject.org/packages/sssd/0.7.1/1.fc12/src/sssd-0.7.1-1.fc12.src.rpm

In /var/log/sssd/sssd.log:
[sssd[be[LDAP]]] [load_backend_module] (0): Unable to load init fn
sssm_ldap_access_init from module ldap, error:
/usr/lib64/sssd/libsss_ldap.so: undefined symbol:
sssm_ldap_access_init

Is this a problem? Authentication appears to be working but I'm not
sure 100% as that can't be good.

-- 
Jeff Schroeder

Don't drink and derive, alcohol and analysis don't mix.
http://www.digitalprognosis.com
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

[SSSD] [PATCH] Add support for option descriptions to SSSDConfig API

2009-10-30 Thread Stephen Gallagher 
Addresses https://fedorahosted.org/sssd/ticket/242 related to
authconfig integration


Note: this applies atop my previous patch "Make config_file_version a
hidden setting in SSSDConfig API" which is still pending review.


-- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
From 85293744afb352b912ff36e750fa4c7107f2d99f Mon Sep 17 00:00:00 2001
From: Stephen Gallagher 
Date: Fri, 30 Oct 2009 15:26:54 -0400
Subject: [PATCH] Add support for option descriptions to SSSDConfig API

Addresses https://fedorahosted.org/sssd/ticket/242 related to
authconfig integration
---
 server/config/SSSDConfig.py  |   41 ++---
 server/config/SSSDConfigTest.py  |8 +-
 server/config/etc/sssd.api.conf  |   65 
 server/config/etc/sssd.api.d/sssd-ipa.conf   |6 +-
 server/config/etc/sssd.api.d/sssd-krb5.conf  |   12 ++--
 server/config/etc/sssd.api.d/sssd-ldap.conf  |  110 +-
 server/config/etc/sssd.api.d/sssd-local.conf |4 +-
 server/config/etc/sssd.api.d/sssd-proxy.conf |4 +-
 8 files changed, 130 insertions(+), 120 deletions(-)

diff --git a/server/config/SSSDConfig.py b/server/config/SSSDConfig.py
index 578f292..8879330 100644
--- a/server/config/SSSDConfig.py
+++ b/server/config/SSSDConfig.py
@@ -25,6 +25,12 @@ class NoSuchProviderError(SSSDConfigException): pass
 class NoSuchProviderSubtypeError(SSSDConfigException): pass
 class ProviderSubtypeInUse(SSSDConfigException): pass
 
+# Indexes
+PRIMARY_TYPE = 0
+SUBTYPE = 1
+DESC = 2
+DEFAULT = 3
+
 class SSSDConfigSchema(RawConfigParser):
 def __init__(self, schemafile, schemaplugindir):
 #TODO: get these from a global setting
@@ -81,33 +87,38 @@ class SSSDConfigSchema(RawConfigParser):
 split_option = self._striplist(unparsed_option.split(','))
 optionlen = len(split_option)
 
-primarytype = self.type_lookup[split_option[0]]
-subtype = self.type_lookup[split_option[1]]
+primarytype = self.type_lookup[split_option[PRIMARY_TYPE]]
+subtype = self.type_lookup[split_option[SUBTYPE]]
+desc = unicode(split_option[DESC])
 
-if optionlen == 2:
+if optionlen == 3:
 # This option has no defaults
 parsed_options[option] = \
 (primarytype,
  subtype,
+ desc,
  None)
-elif optionlen == 3:
-if type(split_option[2]) == primarytype:
+elif optionlen == 4:
+if type(split_option[DEFAULT]) == primarytype:
 parsed_options[option] = \
 (primarytype,
  subtype,
- split_option[2])
+ desc,
+ split_option[DEFAULT])
 elif primarytype == list:
-if (type(split_option[2]) == subtype):
+if (type(split_option[DEFAULT]) == subtype):
 parsed_options[option] = \
 (primarytype,
  subtype,
- [split_option[2]])
+ desc,
+ [split_option[DEFAULT]])
 else:
 try:
 parsed_options[option] = \
 (primarytype,
  subtype,
- [subtype(split_option[2])])
+ desc,
+ [subtype(split_option[DEFAULT])])
 except ValueError:
 raise ParsingError
 else:
@@ -115,15 +126,16 @@ class SSSDConfigSchema(RawConfigParser):
 parsed_options[option] = \
 (primarytype,
  subtype,
- primarytype(split_option[2]))
+ desc,
+ primarytype(split_option[DEFAULT]))
 except ValueError:
 raise ParsingError
 
-elif optionlen > 3:
+elif optionlen > 4:
 if (primarytype != list):
 raise ParsingError
 fixed_options = []
-for x in split_option[2:]:
+for x in split_option[DEFAULT:]:
 if type(x) != subtype:
 try:
 fixed_options.extend([subtype(x)])
@@ -134,6 +146,7 @@ class SSSDConfigSchema(RawConfigParser):
 parsed_options[option] = \
 (primarytype,
  subtype,
+ desc,

Re: [SSSD] [PATCH] fix non-sasl binds in native ldap driver

2009-10-30 Thread Stephen Gallagher 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 10/30/2009 12:13 PM, Simo Sorce wrote:
> My fault for not testing the non-auth case :/
> 
> Simo.
> 
> 
> 
> 
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel

Ack and pushed to master.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkrrEcUACgkQeiVVYja6o6N4igCdH6PVjcsDAgBdHBjE8Y2zkd3q
ioEAoKX0ESLPiiF01OZg4AtKw8WuvJVv
=U0ET
-END PGP SIGNATURE-
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

[SSSD] [PATCH] fix non-sasl binds in native ldap driver

2009-10-30 Thread Simo Sorce 
My fault for not testing the non-auth case :/

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
>From b862c7e203b74ff27bbeb3c2253af352e53ffc42 Mon Sep 17 00:00:00 2001
From: Simo Sorce 
Date: Fri, 30 Oct 2009 12:06:49 -0400
Subject: [PATCH] Fix segfault when SASL is not used at all

---
 server/providers/ldap/sdap.c   |2 ++
 server/providers/ldap/sdap_async.c |4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/server/providers/ldap/sdap.c b/server/providers/ldap/sdap.c
index cefbd5f..5da698a 100644
--- a/server/providers/ldap/sdap.c
+++ b/server/providers/ldap/sdap.c
@@ -395,6 +395,8 @@ bool sdap_rootdse_sasl_mech_is_supported(struct sysdb_attrs *rootdse,
 struct ldb_val *val;
 int i;
 
+if (!sasl_mech) return false;
+
 for (i = 0; i < rootdse->num; i++) {
 if (strcasecmp(rootdse->a[i].name, "supportedSASLMechanisms")) {
 continue;
diff --git a/server/providers/ldap/sdap_async.c b/server/providers/ldap/sdap_async.c
index ae6806f..dfdd267 100644
--- a/server/providers/ldap/sdap_async.c
+++ b/server/providers/ldap/sdap_async.c
@@ -3405,7 +3405,7 @@ static void sdap_cli_connect_done(struct tevent_req *subreq)
 
 sasl_mech = dp_opt_get_string(state->opts->basic, SDAP_SASL_MECH);
 
-if (state->use_rootdse) {
+if (sasl_mech && state->use_rootdse) {
 /* check if server claims to support GSSAPI */
 if (!sdap_rootdse_sasl_mech_is_supported(state->rootdse,
  sasl_mech)) {
@@ -3468,7 +3468,7 @@ static void sdap_cli_rootdse_done(struct tevent_req *subreq)
 
 sasl_mech = dp_opt_get_string(state->opts->basic, SDAP_SASL_MECH);
 
-if (state->use_rootdse) {
+if (sasl_mech && state->use_rootdse) {
 /* check if server claims to support GSSAPI */
 if (!sdap_rootdse_sasl_mech_is_supported(state->rootdse,
  sasl_mech)) {
-- 
1.6.2.5

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

[SSSD] [PATCHES] Fail over helper functions and reference counting wrappers

2009-10-30 Thread Martin Nagy 
Hi,
attached are patches needed for the fail over functionality. The
service discovery is not there yet, I want to hold of with that until I
have at least a basic SRV-based one so I can test it properly. It's
possible that we will discover something missing when we'll be
integrating it into providers. Together with Steven we at least figured
out that for ldapi:// for example we need an "extra" treatment. So I
made the name resolution optional and you can provide a server with
user data. The commit messages and header files should explain it
better.

Martin>From 94a3fe2f29ac354180d37a9767ad4d722a00a3ea Mon Sep 17 00:00:00 2001
From: Martin Nagy 
Date: Tue, 6 Oct 2009 00:28:26 +0200
Subject: [PATCH 1/3] Add DLIST_FOR_EACH() macro

---
 server/util/dlinklist.h |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/server/util/dlinklist.h b/server/util/dlinklist.h
index 5624124..0142a13 100644
--- a/server/util/dlinklist.h
+++ b/server/util/dlinklist.h
@@ -110,4 +110,7 @@ do { \
 		} \
 } while (0)
 
+#define DLIST_FOR_EACH(p, list) \
+	for ((p) = (list); (p) != NULL; (p) = (p)->next)
+
 #endif /* _DLINKLIST_H */
-- 
1.6.2.5

>From 21a5cbec3c01fee2fd1de928569deec0c68d5f30 Mon Sep 17 00:00:00 2001
From: Martin Nagy 
Date: Tue, 6 Oct 2009 01:34:55 +0200
Subject: [PATCH 2/3] Add simple reference counting wrappers for talloc

---
 server/Makefile.am|   16 +++-
 server/tests/refcount-tests.c |  231 +
 server/util/refcount.c|   88 
 server/util/refcount.h|   39 +++
 4 files changed, 373 insertions(+), 1 deletions(-)
 create mode 100644 server/tests/refcount-tests.c
 create mode 100644 server/util/refcount.c
 create mode 100644 server/util/refcount.h

diff --git a/server/Makefile.am b/server/Makefile.am
index 63020c8..afb659f 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -68,7 +68,8 @@ if HAVE_CHECK
 resolv-tests \
 krb5-utils-tests \
 check_and_open-tests \
-files-tests
+files-tests \
+refcount-tests
 endif
 
 check_PROGRAMS = \
@@ -184,6 +185,7 @@ SSSD_UTIL_OBJ = \
 util/strtonum.c \
 util/check_and_open.c \
 util/files.c \
+util/refcount.c \
 $(SSSD_DEBUG_OBJ)
 
 SSSD_RESPONDER_OBJ = \
@@ -247,6 +249,7 @@ dist_noinst_HEADERS = \
 util/util.h \
 util/strtonum.h \
 util/sss_ldap.h \
+util/refcount.h \
 config.h \
 monitor/monitor.h \
 monitor/monitor_interfaces.h \
@@ -453,6 +456,17 @@ resolv_tests_LDADD = \
 $(SSSD_LIBS) \
 $(CHECK_LIBS) \
 $(CARES_LIBS)
+
+refcount_tests_SOURCES = \
+tests/refcount-tests.c \
+tests/common.c \
+$(CHECK_OBJ) \
+$(SSSD_UTIL_OBJ)
+refcount_tests_CFLAGS = \
+$(CHECK_CFLAGS)
+refcount_tests_LDADD = \
+$(SSSD_LIBS) \
+$(CHECK_LIBS)
 endif
 
 
diff --git a/server/tests/refcount-tests.c b/server/tests/refcount-tests.c
new file mode 100644
index 000..ff82b4c
--- /dev/null
+++ b/server/tests/refcount-tests.c
@@ -0,0 +1,231 @@
+/*
+   SSSD
+
+   Reference counting tests.
+
+   Authors:
+Martin Nagy 
+
+   Copyright (C) Red Hat, Inc 2009
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU 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 General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see .
+*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "tests/common.h"
+#include "util/util.h"
+
+/* Interface under test */
+#include "util/refcount.h"
+
+/* Fail the test if object 'obj' does not have 'num' references. */
+#define REF_ASSERT(obj, num) \
+fail_unless(((obj)->DO_NOT_TOUCH_THIS_MEMBER_refcount == (num)), \
+"Reference count of " #obj " should be %d but is %d", \
+(num), (obj)->DO_NOT_TOUCH_THIS_MEMBER_refcount)
+
+#define FILLER_SIZE 32
+
+struct foo {
+REFCOUNT_COMMON;
+char a[FILLER_SIZE];
+char b[FILLER_SIZE];
+};
+
+struct bar {
+char a[FILLER_SIZE];
+REFCOUNT_COMMON;
+char b[FILLER_SIZE];
+};
+
+struct baz {
+char a[FILLER_SIZE];
+char b[FILLER_SIZE];
+REFCOUNT_COMMON;
+};
+
+#define SET_FILLER(target) do { \
+memset((target)->a, 'a', FILLER_SIZE); \
+memset((target)->b, 'b', FILLER_SIZE); \
+} while (0)
+
+#define CHECK_FILLER(target) do { \
+int _counter; \
+for (_counter = 0; _counter < FILLER_SIZE; _counter++) { \
+fail_unless((target)->a[_counter] == 'a', "Corrupted memory in "  \
+

Re: [SSSD] [PATCH] various fixes for tests

2009-10-30 Thread Stephen Gallagher 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 10/30/2009 07:58 AM, Sumit Bose wrote:
> Hi,
> 
> this three patches should fix some issues with the unit tests
> 
> 0001: currently the tests are compiled without AM_CFLAGS, i.e. without
> most of the warning enabled
> 
> 0002: fixes the warnings that were discovered by 0001
> 
> 0003: removes the test sysdb file before the tests are run. Now the
> sysdb test can run successful even if a previous run leaves the ldb file
> in a bad state.
> 
> bye,
> Sumit
> 
> 
> 
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel

Ack and pushed to master.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkrq2EoACgkQeiVVYja6o6PEtgCdGslO2Um1Eo5ZmQzXGqY4BjYS
Tt4AniBSuBKatqz2cHWTbS7MX+I8Z0fi
=CpIH
-END PGP SIGNATURE-
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] start adding an attribute map for generic attributes

2009-10-30 Thread Stephen Gallagher 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 10/30/2009 04:44 AM, Sumit Bose wrote:
> On Thu, Oct 29, 2009 at 06:20:48PM -0400, Simo Sorce wrote:
>> While adding infrastructure to use the USN counter for enumerations I
>> found that sdap_id_map is not really a good name as the map represent a
>> generic attribute map. This patch renames the structure and adds some
>> generic infrastructure for non user/group specific attribute mappings.
>>
>> This infrastructure is not used yet, will be in the next patches.
>>
> 
> I expect that the updates to config/etc/sssd.api.d/sssd-ldap.conf are in
> the next patch :-), but they should be in this one. Otherwise the patch
> works fine for me.

Agreed, please include the API config changes in this patch.

> 
> bye,
> Sumit
> 
>> Simo.
>>
>> -- 
>> Simo Sorce * Red Hat, Inc * New York
> 
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel


- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkrq1nsACgkQeiVVYja6o6NDiACgkin0UronKS67u80TAel7ZXjk
QNEAnRlrsEbj+qUYy6KV2w7yhQXa5QD/
=om0d
-END PGP SIGNATURE-
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

[SSSD] [PATCH] various fixes for tests

2009-10-30 Thread Sumit Bose 
Hi,

this three patches should fix some issues with the unit tests

0001: currently the tests are compiled without AM_CFLAGS, i.e. without
most of the warning enabled

0002: fixes the warnings that were discovered by 0001

0003: removes the test sysdb file before the tests are run. Now the
sysdb test can run successful even if a previous run leaves the ldb file
in a bad state.

bye,
Sumit
>From a03606499931ef0de20e2d28f57580396e25314a Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Fri, 30 Oct 2009 11:25:55 +0100
Subject: [PATCH 1/3] Add AM_CFLAGS to unit tests

---
 server/Makefile.am |   20 
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/server/Makefile.am b/server/Makefile.am
index 63020c8..81223f4 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -380,6 +380,7 @@ sysdb_tests_SOURCES = \
 tests/sysdb-tests.c \
 $(SSSD_UTIL_OBJ)
 sysdb_tests_CFLAGS = \
+$(AM_CFLAGS) \
 $(CHECK_CFLAGS)
 sysdb_tests_LDADD = \
 $(SSSD_LIBS) \
@@ -390,6 +391,7 @@ strtonum_tests_SOURCES = \
 util/debug.c \
 util/strtonum.c
 strtonum_tests_CFLAGS = \
+$(AM_CFLAGS) \
 $(CHECK_CFLAGS)
 strtonum_tests_LDADD = \
 $(SSSD_LIBS) \
@@ -402,6 +404,7 @@ krb5_utils_tests_SOURCES = \
 providers/data_provider_opts.c \
 $(SSSD_UTIL_OBJ)
 krb5_utils_tests_CFLAGS = \
+$(AM_CFLAGS) \
 $(CHECK_CFLAGS)
 krb5_utils_tests_LDADD = \
 $(SSSD_LIBS)\
@@ -412,6 +415,7 @@ check_and_open_tests_SOURCES = \
 tests/check_and_open-tests.c \
 util/check_and_open.c
 check_and_open_tests_CFLAGS = \
+$(AM_CFLAGS) \
 $(CHECK_CFLAGS)
 check_and_open_tests_LDADD = \
 $(CHECK_LIBS)
@@ -430,24 +434,18 @@ files_tests_SOURCES = \
 util/check_and_open.c \
 util/files.c
 files_tests_CFLAGS = \
+$(AM_CFLAGS) \
 $(CHECK_CFLAGS)
 files_tests_LDADD = \
 $(FILES_TESTS_LIBS)
-endif
-
-stress_tests_SOURCES = \
-tests/stress-tests.c \
-$(SSSD_UTIL_OBJ)
-stress_tests_LDADD = \
-$(SSSD_LIBS)
 
-if HAVE_CHECK
 resolv_tests_SOURCES = \
 tests/resolv-tests.c \
 tests/common.c \
 $(SSSD_UTIL_OBJ) \
 $(SSSD_RESOLV_OBJ)
 resolv_tests_CFLAGS = \
+$(AM_CFLAGS) \
 $(CHECK_CFLAGS)
 resolv_tests_LDADD = \
 $(SSSD_LIBS) \
@@ -455,6 +453,12 @@ resolv_tests_LDADD = \
 $(CARES_LIBS)
 endif
 
+stress_tests_SOURCES = \
+tests/stress-tests.c \
+$(SSSD_UTIL_OBJ)
+stress_tests_LDADD = \
+$(SSSD_LIBS)
+
 
 # Plugin Libraries #
 
-- 
1.6.2.5

>From 3e6a6085ae4e3b51692605d316cd183008e781b4 Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Fri, 30 Oct 2009 11:26:50 +0100
Subject: [PATCH 2/3] Fix compiler warnings in krb5_utils-tests.

---
 server/tests/krb5_utils-tests.c |   42 +++---
 1 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/server/tests/krb5_utils-tests.c b/server/tests/krb5_utils-tests.c
index b79cb7c..b0f6a1d 100644
--- a/server/tests/krb5_utils-tests.c
+++ b/server/tests/krb5_utils-tests.c
@@ -63,7 +63,7 @@ void setup_talloc_context(void)
 krb5_ctx = talloc_zero(tmp_ctx, struct krb5_ctx);
 fail_unless(pd != NULL, "Cannot create krb5_ctx structure.");
 
-pd->user = USERNAME;
+pd->user = discard_const(USERNAME);
 pd->pw_uid = atoi(UID);
 pd->upn = PRINCIPLE_NAME;
 pd->cli_pid = atoi(PID);
@@ -97,8 +97,8 @@ void free_talloc_context(void)
 
 START_TEST(test_multiple_substitutions)
 {
-char *test_template = BASE"_%u_%U_%u";
-char *expected = BASE"_"USERNAME"_"UID"_"USERNAME;
+const char *test_template = BASE"_%u_%U_%u";
+const char *expected = BASE"_"USERNAME"_"UID"_"USERNAME;
 char *result;
 
 result = expand_ccname_template(tmp_ctx, kr, test_template);
@@ -112,8 +112,8 @@ END_TEST
 
 START_TEST(test_username)
 {
-char *test_template = BASE"_%u";
-char *expected = BASE"_"USERNAME;
+const char *test_template = BASE"_%u";
+const char *expected = BASE"_"USERNAME;
 char *result;
 
 result = expand_ccname_template(tmp_ctx, kr, test_template);
@@ -127,8 +127,8 @@ END_TEST
 
 START_TEST(test_uid)
 {
-char *test_template = BASE"_%U";
-char *expected = BASE"_"UID;
+const char *test_template = BASE"_%U";
+const char *expected = BASE"_"UID;
 char *result;
 
 result = expand_ccname_template(tmp_ctx, kr, test_template);
@@ -142,8 +142,8 @@ END_TEST
 
 START_TEST(test_upn)
 {
-char *test_template = BASE"_%p";
-char *expected = BASE"_"PRINCIPLE_NAME;
+const char *test_template = BASE"_%p";
+const char *expected = BASE"_"PRINCIPLE_NAME;
 char *result;
 
 result = expand_ccname_template(tmp_ctx, kr, test_template);
@@ -157,8 +157,8 @@ END_TEST
 
 START_TEST(test_realm)
 {
-char *test_template = BASE"_%r";
-char *expected = BASE"_"REALM;
+const char *test_template = BASE"_%r";
+const char *expected = BASE"_"REALM;
 char *result;
 
 result = expand_ccname_template(tmp_ctx, kr, test_

Re: [SSSD] [PATCH] add sysdb_delete_recursive request to sysdb API

2009-10-30 Thread Sumit Bose 
On Thu, Oct 29, 2009 at 11:26:39PM +0100, Sumit Bose wrote:
> On Thu, Oct 29, 2009 at 09:32:34PM +, Simo Sorce wrote:
> > On Thu, 2009-10-29 at 19:40 +0100, Sumit Bose wrote:
> > > On Thu, Oct 29, 2009 at 01:39:21PM +0100, Sumit Bose wrote:
> > > > Hi,
> > > > 
> > > > this patch adds a recursive delete request to the sysdb API. It has
> > > the
> > > > same interface as sysdb_delete_entry, but does not delete the entry,
> > > but
> > > > its children.
> > > > 
> > > > bye,
> > > > Sumit
> > > 
> > > This is a new version of the patch which tries to delete the entry AND
> > > all its children. It searches all objects with a subtree search, sorts
> > > the result so that the ones with the most components come first and
> > > finally loops over the results and deletes them.
> > 
> > Comments inline.
> > 
> > > +
> > > +subreq = sysdb_search_entry_send(state, ev, handle, dn,
> > > LDB_SCOPE_SUBTREE,
> > > + "distinguishedName=*", NULL);
> > 
> > Please use "(objectclass=*)" as filter to catch all entries.
> > 
> 
> I would prefer to stay with distinguishedName, because it is
> auto-generated and always present.
> 
> > Also please set attrs. Passing NULL, means you will retrieve all
> > attributes wasting a lot of memory unnecessarily. You are interested
> > only in the entries msg->dn, so you probably do not want any attribute
> > returned at all.
> 
> ah, I thought NULL means nothing, now I pass { NULL }
> 
> > 
> > [..]
> > 
> > > +static int compare_ldb_dn_comp_num(const void *m1, const void *m2)
> > > +{
> > > +struct ldb_message *msg1 = talloc_get_type(*(const void **) m1,
> > > +   struct ldb_message);
> > > +struct ldb_message *msg2 = talloc_get_type(*(const void **) m2,
> > > +   struct ldb_message);
> > > +
> > > +return ldb_dn_get_comp_num(msg2->dn) -
> > > ldb_dn_get_comp_num(msg1->dn);
> > > +}
> > 
> > Please move this function in sysdb.c, it's a generic function that can
> > be used by multiple functions and here just interrupts reading the
> > program flow.
> 
> done
> 
> > 
> > > +static void sysdb_delete_recursive_loop(struct tevent_req *subreq)
> > [...]
> > 
> > I think you should split the this function into a function that receives
> > the results of sysdb_search_entry_recv() and then another one that sets
> > the loop. If necessary use the trick I used in sdap_cli_connect to do
> > continuation functions (see the sdap_cli_*_step functions).
> > 
> 
> done
> 
> > The rest looks good to me.
> > 
> 
> Thanks for reviewing.
> 
> bye,
> Sumit
> 

sorry, this new patch fixes a compiler warning.

bye,
Sumit
>From 5f7ad391b2d4626115f110af6269a5577d154022 Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Thu, 29 Oct 2009 12:57:57 +0100
Subject: [PATCH] add sysdb_delete_recursive request to sysdb API

---
 server/db/sysdb.c  |   12 +++
 server/db/sysdb.h  |   10 +++
 server/db/sysdb_ops.c  |  159 
 server/tests/sysdb-tests.c |  111 +-
 4 files changed, 288 insertions(+), 4 deletions(-)

diff --git a/server/db/sysdb.c b/server/db/sysdb.c
index 5811ddc..a2ac3b2 100644
--- a/server/db/sysdb.c
+++ b/server/db/sysdb.c
@@ -1417,3 +1417,15 @@ int sysdb_get_ctx_from_list(struct sysdb_ctx_list 
*ctx_list,
 /* definitely not found */
 return ENOENT;
 }
+
+
+int compare_ldb_dn_comp_num(const void *m1, const void *m2)
+{
+struct ldb_message *msg1 = talloc_get_type(*(void **) discard_const(m1),
+   struct ldb_message);
+struct ldb_message *msg2 = talloc_get_type(*(void **) discard_const(m2),
+   struct ldb_message);
+
+return ldb_dn_get_comp_num(msg2->dn) - ldb_dn_get_comp_num(msg1->dn);
+}
+
diff --git a/server/db/sysdb.h b/server/db/sysdb.h
index 00a3378..72f56db 100644
--- a/server/db/sysdb.h
+++ b/server/db/sysdb.h
@@ -190,6 +190,8 @@ struct ldb_dn *sysdb_custom_dn(struct sysdb_ctx *ctx, void 
*memctx,
 struct ldb_context *sysdb_ctx_get_ldb(struct sysdb_ctx *ctx);
 struct ldb_context *sysdb_handle_get_ldb(struct sysdb_handle *handle);
 
+int compare_ldb_dn_comp_num(const void *m1, const void *m2);
+
 /* function to start and finish a transaction
  * sysdb_transaction_send() will queue a request for a transaction
  * when it is done it will call the tevent_req callback, which must
@@ -311,6 +313,14 @@ struct tevent_req *sysdb_delete_entry_send(TALLOC_CTX 
*mem_ctx,
bool ignore_not_found);
 int sysdb_delete_entry_recv(struct tevent_req *req);
 
+
+struct tevent_req *sysdb_delete_recursive_send(TALLOC_CTX *mem_ctx,
+   struct tevent_context *ev,
+   struct sysdb_handle *handle,
+   struct ldb_dn *dn,

Re: [SSSD] [PATCH] start adding an attribute map for generic attributes

2009-10-30 Thread Sumit Bose 
On Thu, Oct 29, 2009 at 06:20:48PM -0400, Simo Sorce wrote:
> While adding infrastructure to use the USN counter for enumerations I
> found that sdap_id_map is not really a good name as the map represent a
> generic attribute map. This patch renames the structure and adds some
> generic infrastructure for non user/group specific attribute mappings.
> 
> This infrastructure is not used yet, will be in the next patches.
> 

I expect that the updates to config/etc/sssd.api.d/sssd-ldap.conf are in
the next patch :-), but they should be in this one. Otherwise the patch
works fine for me.

bye,
Sumit

> Simo.
> 
> -- 
> Simo Sorce * Red Hat, Inc * New York

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel