<div dir="ltr">Hi, Lukas<br><div class="gmail_extra"><br><br><div class="gmail_quote">2013/10/11 Lukas Slebodnik <span dir="ltr">&lt;<a href="mailto:lslebodn@redhat.com" target="_blank">lslebodn@redhat.com</a>&gt;</span><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On (11/10/13 12:53), Benjamin Franzke wrote:<br>
&gt;2013/10/11 Sumit Bose &lt;<a href="mailto:sbose@redhat.com">sbose@redhat.com</a>&gt;<br>
&gt;<br>
&gt;&gt; On Thu, Oct 10, 2013 at 02:41:54PM +0200, Benjamin Franzke wrote:<br>
&gt;&gt; &gt; Thanks for your review!<br>
&gt;&gt; &gt;<br>
&gt;&gt; &lt;snip&gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; The patch with your concers fixed is attached.<br>
&gt;&gt;<br>
&gt;&gt; Thank you for the new version. I only have two comments left:<br>
&gt;&gt;<br>
&gt;&gt; +    if (handle == NULL || errmsg == NULL) {<br>
&gt;&gt; +        *errmsg = strerror(EINVAL);<br>
&gt;&gt;<br>
&gt;&gt; you cannot return an error message if errmsg == NULL.<br>
&gt;&gt;<br>
&gt;<br>
&gt;yes, of course not :)<br>
&gt;had to leave house and wanted to get the patch out before, that dumb<br>
&gt;mistake was the result :\<br>
&gt;<br>
&gt;&gt;<br>
&gt;&gt; +        return -EINVAL;<br>
&gt;&gt; +    }<br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;&gt; +    err = sss_nss_getsidbyname(name, &amp;sid, &amp;id_type);<br>
&gt;&gt; +    if (err != 0)  {<br>
&gt;&gt; +        /* Might be a raw string representation of SID,<br>
&gt;&gt; +         * try converting that before returning an error. */<br>
&gt;&gt; +        if (sid_to_cifs_sid(ctx, name, csid) == 0)<br>
&gt;&gt; +            return 0;<br>
&gt;&gt; +<br>
&gt;&gt;<br>
&gt;&gt;  ^^^^^^^^ trailing whitespace<br>
&gt;&gt;<br>
&gt;&gt; Oops, again.. have them highlighted in my editor now ;)<br>
&gt;<br>
&gt;&gt; +        ctx_set_error(ctx, strerror(err));<br>
&gt;&gt; +        return -err;<br>
&gt;&gt; +    }<br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;&gt; bye,<br>
&gt;&gt; Sumit<br>
&gt;&gt;<br>
&gt;<br>
&gt;New patch attached, with these two things fixed.<br>
<br>
</div></div>Inline.<br>
<br>
>From c548048ad7f7db530de2194d9a71ba7a265a34c4 Mon Sep 17 00:00:00 2001<br>
From: Benjamin Franzke &lt;<a href="mailto:benjaminfranzke@googlemail.com">benjaminfranzke@googlemail.com</a>&gt;<br>
Date: Thu, 26 Sep 2013 10:27:33 +0200<br>
Subject: [PATCH] Add CIFS idmap plugin<br>
<br>
---<br>
 Makefile.am                             |  21 ++<br>
 <a href="http://configure.ac" target="_blank">configure.ac</a>                            |   2 +<br>
 src/conf_macros.m4                      |  14 ++<br>
 src/external/cifsidmap.m4               |  13 ++<br>
 src/lib/cifs_idmap_sss/cifs_idmap_sss.c | 339 ++++++++++++++++++++++++++++++++<br>
 5 files changed, 389 insertions(+)<br>
 create mode 100644 src/external/cifsidmap.m4<br>
 create mode 100644 src/lib/cifs_idmap_sss/cifs_idmap_sss.c<br>
<br>
diff --git a/src/external/cifsidmap.m4 b/src/external/cifsidmap.m4<br>
new file mode 100644<br>
index 0000000000000000000000000000000000000000..546c373b0e66dd4136bca8d92cf220102b2f7830<br>
--- /dev/null<br>
+++ b/src/external/cifsidmap.m4<br>
@@ -0,0 +1,13 @@<br>
+AC_ARG_ENABLE([cifs-idmap-plugin],<br>
+              [AS_HELP_STRING([--disable-cifs-idmap-plugin],<br>
+                              [do not build CIFS idmap plugin])],<br>
+              [build_cifs_idmap_plugin=$enableval],<br>
+              [build_cifs_idmap_plugin=yes])<br>
                                      ^^^^^^<br>
This line mean that cifs_idmap_plugin will be build by default,<br>
but <a href="http://sssd.spec.in" target="_blank">sssd.spec.in</a> file is not updated with appropriate devel package.<br></blockquote><div><br></div><div>As Sumit said, he&#39;ll do that part.. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
Something like<br>
  %if (0%{?fedora} &gt;= 17)<br>
  BuildRequires: cifs-utils-devel<br>
  %endif<br>
<br>
BTW: cifs-utils-devel is not available on RHEL6, do we want to build<br>
cifs_idmap_plugin by default?<br>
<br>
And library cifs_idmap_sss.so should be added to some (or new) subpackage<br>
in spec file. If you are not familiar with rpm packaging, we can help you.<br>
<br>
+<br>
+AS_IF([test x$build_cifs_idmap_plugin = xyes],<br>
+      [AC_CHECK_HEADER([cifsidmap.h], [],<br>
+                       [AC_MSG_ERROR([you must have the cifsidmap header installed to build the idmap plugin])])<br>
+      ])<br>
+<br>
+AM_CONDITIONAL([BUILD_CIFS_IDMAP_PLUGIN],<br>
+               [test x$build_cifs_idmap_plugin = xyes])<br>
<div class="im"><br>
<br>
diff --git a/src/lib/cifs_idmap_sss/cifs_idmap_sss.c b/src/lib/cifs_idmap_sss/cifs_idmap_sss.c<br>
new file mode 100644<br>
</div>index 0000000000000000000000000000000000000000..bcea6d6d84dbd6184c2f719d0c1b4a2ac3c014ca<br>
--- /dev/null<br>
+++ b/src/lib/cifs_idmap_sss/cifs_idmap_sss.c<br>
<br>
... snip ...<br>
<div class="im"><br>
+static int samba_unix_sid_to_id(const char *sid, struct cifs_uxid *cuxid)<br>
+{<br>
+    id_t id;<br>
+    uint8_t type;<br>
+<br>
+    if (sscanf(sid, &quot;S-1-22-%hhu-%u&quot;, &amp;type, &amp;id) != 2)<br>
+        return -1;<br>
+<br>
+    switch (type) {<br>
+    case 1:<br>
+        cuxid-&gt;type = CIFS_UXID_TYPE_UID;<br>
+        cuxid-&gt;id.uid = id;<br>
+        break;<br>
+    case 2:<br>
+        cuxid-&gt;type = CIFS_UXID_TYPE_GID;<br>
+        cuxid-&gt;id.gid = id;<br>
</div>        ^^^^^^^<br>
Missing brake statement.<br>
Is it aim or mistake?<br></blockquote><div><br></div><div>That was a mistake, thanks. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
+    default:<br>
+        cuxid-&gt;type = CIFS_UXID_TYPE_UNKNOWN;<br>
+        return -1;<br>
+    }<br>
+<br>
+    return 0;<br>
+}<br>
<br>
</div>And the last note.<br>
You created new dynamic library. Could you add this library to dlopen test?<br></blockquote><div><br></div><div>Done. New patch attached. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
+++ b/src/tests/dlopen-tests.c<br>
@@ -40,6 +40,7 @@ struct so {<br>
     { &quot;libsss_debug.so&quot;, { LIBPFX&quot;libsss_debug.so&quot;, NULL } },<br>
     { &quot;libipa_hbac.so&quot;, { LIBPFX&quot;libipa_hbac.so&quot;, NULL } },<br>
     { &quot;libsss_idmap.so&quot;, { LIBPFX&quot;libsss_idmap.so&quot;, NULL } },<br>
+    { &quot;cifs_idmap_sss.so&quot;, { LIBPFX&quot;cifs_idmap_sss.so&quot;, NULL } },<br>
     { &quot;libsss_nss_idmap.so&quot;, { LIBPFX&quot;libsss_nss_idmap.so&quot;, NULL } },<br>
     { &quot;libnss_sss.so&quot;, { LIBPFX&quot;libnss_sss.so&quot;, NULL } },<br>
     { &quot;pam_sss.so&quot;, { LIBPFX&quot;pam_sss.so&quot;, NULL } }, </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thank you very much .<br>
<span class="HOEnZb"><font color="#888888"><br>
LS<br>
</font></span><div class="HOEnZb"><div class="h5">_______________________________________________<br>
sssd-devel mailing list<br>
<a href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a><br>
<a href="https://lists.fedorahosted.org/mailman/listinfo/sssd-devel" target="_blank">https://lists.fedorahosted.org/mailman/listinfo/sssd-devel</a><br>
</div></div></blockquote></div><br></div></div>