[SSSD] [PATCH 1/2] libsss_idmap: function to calculate range

Dmitri Pal dpal at redhat.com
Tue Apr 23 15:32:39 UTC 2013


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 at 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/



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130423/5fb386e6/attachment.html>


More information about the sssd-devel mailing list