<div dir="ltr">Hi, Lukas<br><div class="gmail_extra"><br><br><div class="gmail_quote">2013/10/11 Lukas Slebodnik <span dir="ltr"><<a href="mailto:lslebodn@redhat.com" target="_blank">lslebodn@redhat.com</a>></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>
>2013/10/11 Sumit Bose <<a href="mailto:sbose@redhat.com">sbose@redhat.com</a>><br>
><br>
>> On Thu, Oct 10, 2013 at 02:41:54PM +0200, Benjamin Franzke wrote:<br>
>> > Thanks for your review!<br>
>> ><br>
>> <snip><br>
>> ><br>
>> > The patch with your concers fixed is attached.<br>
>><br>
>> Thank you for the new version. I only have two comments left:<br>
>><br>
>> + if (handle == NULL || errmsg == NULL) {<br>
>> + *errmsg = strerror(EINVAL);<br>
>><br>
>> you cannot return an error message if errmsg == NULL.<br>
>><br>
><br>
>yes, of course not :)<br>
>had to leave house and wanted to get the patch out before, that dumb<br>
>mistake was the result :\<br>
><br>
>><br>
>> + return -EINVAL;<br>
>> + }<br>
>><br>
>><br>
>> + err = sss_nss_getsidbyname(name, &sid, &id_type);<br>
>> + if (err != 0) {<br>
>> + /* Might be a raw string representation of SID,<br>
>> + * try converting that before returning an error. */<br>
>> + if (sid_to_cifs_sid(ctx, name, csid) == 0)<br>
>> + return 0;<br>
>> +<br>
>><br>
>> ^^^^^^^^ trailing whitespace<br>
>><br>
>> Oops, again.. have them highlighted in my editor now ;)<br>
><br>
>> + ctx_set_error(ctx, strerror(err));<br>
>> + return -err;<br>
>> + }<br>
>><br>
>><br>
>> bye,<br>
>> Sumit<br>
>><br>
><br>
>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 <<a href="mailto:benjaminfranzke@googlemail.com">benjaminfranzke@googlemail.com</a>><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'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} >= 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, "S-1-22-%hhu-%u", &type, &id) != 2)<br>
+ return -1;<br>
+<br>
+ switch (type) {<br>
+ case 1:<br>
+ cuxid->type = CIFS_UXID_TYPE_UID;<br>
+ cuxid->id.uid = id;<br>
+ break;<br>
+ case 2:<br>
+ cuxid->type = CIFS_UXID_TYPE_GID;<br>
+ cuxid->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->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>
{ "libsss_debug.so", { LIBPFX"libsss_debug.so", NULL } },<br>
{ "libipa_hbac.so", { LIBPFX"libipa_hbac.so", NULL } },<br>
{ "libsss_idmap.so", { LIBPFX"libsss_idmap.so", NULL } },<br>
+ { "cifs_idmap_sss.so", { LIBPFX"cifs_idmap_sss.so", NULL } },<br>
{ "libsss_nss_idmap.so", { LIBPFX"libsss_nss_idmap.so", NULL } },<br>
{ "libnss_sss.so", { LIBPFX"libnss_sss.so", NULL } },<br>
{ "pam_sss.so", { LIBPFX"pam_sss.so", 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>