<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>
      &gt; The first patch solves ticket<br>
      &gt; <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>
      &gt; test suite to reflect these changes.<br>
      <br>
      &gt; The patch moves calculation of UID range for a domain from<br>
      &gt; sdap_idmap code to sss_idmap. Some changes are:<br>
      <br>
      &gt; - Structure sdap_idmap_slice has been removed. The slice
      number is<br>
      &gt; now stored in the sss_idmap_range structure.<br>
      <br>
      &gt; - The sss_idmap_init() function takes a new parameter:
      structure <br>
      &gt; sss_idmap_opts. It holds configurable options (UID bounds,<br>
      &gt; rangesize, use autorid mode). These values are also
      accessible<br>
      &gt; outside sss_idmap (simple getters have been added).<br>
      <br>
      &gt; - Options dp_opt_get_int for <br>
      &gt; SDAP_IDMAP_LOWER/UPPER/AUTORID_COMPAT/RANGESIZE was moved
      from <br>
      &gt; sdap_idmap_add_domain to sdap_idmap_init. These values are
      shared<br>
      &gt; among subdomains, so there is no need to call it in each<br>
      &gt; sdap_idmap_add_domain call.<br>
      <br>
      &gt; - sss_idmap_calculate_range function is mostly copy of the
      range <br>
      &gt; calculation processing in sdap_idmap_add_domain(), but it
      uses<br>
      &gt; values stored in sss_idmap_opts and the slice number is taken
      from<br>
      &gt; the rnge structure (which is completely created by this
      function<br>
      &gt; now).<br>
      <br>
      &gt; Sumit, could you please review this patch? I think you are
      most<br>
      &gt; familiar with the id mapping code.<br>
      <br>
      &gt; Also one more question. How should I amend the -version-info
      value<br>
      &gt; for libsss_idmap in Makefile.am? Currently it is 0:1:0.
      Should it<br>
      &gt; 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;">&gt;
      _______________________________________________<br>
      &gt; sssd-devel mailing list<br>
      &gt; <a class="moz-txt-link-abbreviated" href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a><br>
      &gt; <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>