Hi,
find updated patches attached. (Rebased against current master)
Am Donnerstag 23 September 2010, 20:02:20 schrieb Stephen Gallagher:
On 09/20/2010 11:13 AM, Ralf Haferkamp wrote:
[..]
Patch 0001: Ack. This looks fine to me.
Patch 0002: Nack.
There are still a few style issues.
Please move the struct sdap_process_group_state definition to right
before the sdap_get_groups_process() implementation.
Fixed.
I'm not a huge fan of firing off all of the
sdap_process_group_state()
calls in a loop for (effectively) simultaneous operation. I'd prefer
if you implemented this with our _step() approach (where you handle
each one serially until they're all finished). It's quite a bit
easier to follow in a debugger. This will also eliminate the need for
GROUPMEMBER_REQ_PARALLEL. Believe it or not, it won't be any slower
because the LDAP id op operations will serialize the requests anyway
so we don't open large numbers of separate connections to the LDAP
server.
I didn't change anything here. For the reasons detailed in my other
mail.
There's a memory hierarchy bug in sdap_process_group_send(). You
allocate sysdb_dn on memctx, but it really should be a child of
grp_state->sysdb_dns->values[grp_state->sysdb_dns->num_values].
I guess
you ment grp_state->sysdb_dns->values here? Because
grp_state->sysdb_dns->values[grp_state->sysdb_dns->num_values] is a
struct ldb_message_element (no pointer), so it's not usable as a talloc
context. If yes -> Fixed.
I think there's a lot of potential here. We just need to work on
cleaning things up a little bit further.
Patch 0003: Nack.
I agree with Simo that it would make more sense to use a nesting level
rather than a boolean for this task.
I've dropped this patch for now.
Also, any changes to the LDAP options need to be added to
src/config/etc/sssd.api.d/sssd-ldap.conf and have a translatable
string added to src/config/SSSDConfig.py and also have the same
option mirrored in the IPA backend. This latter you will be alerted
to if you attempt to run 'make check'.
--
Ralf