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