On Sat, Jan 28, 2012 at 02:10:19PM -0500, Stephen Gallagher wrote:
On Sat, 2012-01-28 at 13:52 -0500, Stephen Gallagher wrote:
> On Sat, 2012-01-28 at 12:08 -0500, Stephen Gallagher wrote:
> > I'm still working on the enumeration support, but I wanted to get the
> > direct lookup on the list early.
> >
> > Patch 0001: extend sysdb_store_service() to accept additional attributes
> > This is necessary to be able to store provider-specific data in the
> > entry (such as the entryUSN).
Can you move the NULL check of msg->dn outside the switch so that there
is only one and all cases are checked? Currently the service case does
not have a NULL check.
You should also pass primary_name, not name into the call of
sysdb_remove_attrs() because name points to ldb_message structure that
was already freed.
> >
> > Patch 0002: Add a helper function to get an integer value from a
> > sysdb_attrs object directly as a uint16_t.
> >
Ack
> > Patch 0003: The big one. This patch adds support for
looking up services
> > in LDAP. There shouldn't be any big "gotchas" here. It's
very
> > straightforward (if long).
> >
Setting errno to 0 before strtouint16() is not needed, strtouint16()
does it internally.
Otherwise Ack.
Patch 0004: LDAP: Add enumeration support for services
Ack
> > Patch 0004: Extend this support to the IPA Provider (even
though IPA
> > itself does not yet support the services map natively).
>
(This is actually patch 0005 in the most recent set)
Ack
> I over-estimated the additional effort necessary to add the
enumeration
> support. New patch set includes the enumeration work (before the IPA
> provider) as well as fixing a minor typo in patch 0003 (bad copy-paste
> in a comment).
One more set. I forgot to include updates to the SSSDConfig API and the
manpages in the previous set. I added it as a new, sixth patch. This
should now be complete.
Patch 0006: I don't understand this line:
#replaced by ldap_entry_usn# 'ldap_service_entry_usn' : _('Service entryUSN
attribute'),
It looks like SDAP_AT_SERVICE_USN is used in the sdap services code?
Also "ldap_service_search_base" is missing from the manual page.