On 04/23/2013 10:59 AM, Stephen Gallagher wrote: > On 04/23/2013 10:37 AM, Michal Židek wrote: > > The first patch solves ticket > > https://fedorahosted.org/sssd/ticket/1844 The second modifies the > > test suite to reflect these changes. > > > The patch moves calculation of UID range for a domain from > > sdap_idmap code to sss_idmap. Some changes are: > > > - Structure sdap_idmap_slice has been removed. The slice number is > > now stored in the sss_idmap_range structure. > > > - The sss_idmap_init() function takes a new parameter: structure > > sss_idmap_opts. It holds configurable options (UID bounds, > > rangesize, use autorid mode). These values are also accessible > > outside sss_idmap (simple getters have been added). > > > - Options dp_opt_get_int for > > SDAP_IDMAP_LOWER/UPPER/AUTORID_COMPAT/RANGESIZE was moved from > > sdap_idmap_add_domain to sdap_idmap_init. These values are shared > > among subdomains, so there is no need to call it in each > > sdap_idmap_add_domain call. > > > - sss_idmap_calculate_range function is mostly copy of the range > > calculation processing in sdap_idmap_add_domain(), but it uses > > values stored in sss_idmap_opts and the slice number is taken from > > the rnge structure (which is completely created by this function > > now). > > > Sumit, could you please review this patch? I think you are most > > familiar with the id mapping code. > > > Also one more question. How should I amend the -version-info value > > for libsss_idmap in Makefile.am? Currently it is 0:1:0. Should it > > be 1:0:0 now? > > > > Nack. > > First, please don't do the shallow copy in sss_idmap_init(). Just > because the idmap_opts object doesn't have any pointers today doesn't > mean we aren't going to be introducing a hard-to-catch bug later on. > Please make a copy routine (that just does a shallow copy today). > > > I think I just noticed a bug in my original design of the hash > collision code (though it's hopefully small enough that no one has hit > it). The comparison for the loop around back to slice 0 on collision > is off-by-one. If the new_slice == max_slices, it would be choosing a > slice number one past upper range. We should fix that here. Please > double-check my thought process to make sure I'm not getting it wrong. > >
If we confirm that a fix is needed please file a ticket. > > > Build fails: > CC src/responder/ssh/sshsrv_dp.o > CC src/responder/ssh/sshsrv_cmd.o > CC src/responder/pac/sssd_pac-pacsrv.o > CC src/responder/pac/sssd_pac-pacsrv_cmd.o > ../src/responder/pac/pacsrv.c: In function ‘pac_process_init’: > ../src/responder/pac/pacsrv.c:190:26: warning: passing argument 4 of > ‘sss_idmap_init’ from incompatible pointer type [enabled by default] > In file included from ../src/responder/pac/pacsrv.h:41:0, > from ../src/responder/pac/pacsrv.c:35: > ../src/lib/idmap/sss_idmap.h:151:23: note: expected ‘struct > sss_idmap_opts *’ but argument is of type ‘struct sss_idmap_ctx **’ > ../src/responder/pac/pacsrv.c:190:26: error: too few arguments to > function ‘sss_idmap_init’ > In file included from ../src/responder/pac/pacsrv.h:41:0, > from ../src/responder/pac/pacsrv.c:35: > ../src/lib/idmap/sss_idmap.h:151:23: note: declared here > make[1]: *** [src/responder/pac/sssd_pac-pacsrv.o] Error 1 > make[1]: *** Waiting for unfinished jobs.... > make[1]: Leaving directory `/home/sgallagh/workspace/sssd/x86_64' > make: *** [check-recursive] Error 1 > > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
