Hi, Lukas
2013/10/11 Lukas Slebodnik <lslebodn(a)redhat.com>
On (11/10/13 12:53), Benjamin Franzke wrote:
>2013/10/11 Sumit Bose <sbose(a)redhat.com>
>
>> On Thu, Oct 10, 2013 at 02:41:54PM +0200, Benjamin Franzke wrote:
>> > Thanks for your review!
>> >
>> <snip>
>> >
>> > The patch with your concers fixed is attached.
>>
>> Thank you for the new version. I only have two comments left:
>>
>> + if (handle == NULL || errmsg == NULL) {
>> + *errmsg = strerror(EINVAL);
>>
>> you cannot return an error message if errmsg == NULL.
>>
>
>yes, of course not :)
>had to leave house and wanted to get the patch out before, that dumb
>mistake was the result :\
>
>>
>> + return -EINVAL;
>> + }
>>
>>
>> + err = sss_nss_getsidbyname(name, &sid, &id_type);
>> + if (err != 0) {
>> + /* Might be a raw string representation of SID,
>> + * try converting that before returning an error. */
>> + if (sid_to_cifs_sid(ctx, name, csid) == 0)
>> + return 0;
>> +
>>
>> ^^^^^^^^ trailing whitespace
>>
>> Oops, again.. have them highlighted in my editor now ;)
>
>> + ctx_set_error(ctx, strerror(err));
>> + return -err;
>> + }
>>
>>
>> bye,
>> Sumit
>>
>
>New patch attached, with these two things fixed.
Inline.
From c548048ad7f7db530de2194d9a71ba7a265a34c4 Mon Sep 17 00:00:00 2001
From: Benjamin Franzke <benjaminfranzke(a)googlemail.com>
Date: Thu, 26 Sep 2013 10:27:33 +0200
Subject: [PATCH] Add CIFS idmap plugin
---
Makefile.am | 21 ++
configure.ac | 2 +
src/conf_macros.m4 | 14 ++
src/external/cifsidmap.m4 | 13 ++
src/lib/cifs_idmap_sss/cifs_idmap_sss.c | 339
++++++++++++++++++++++++++++++++
5 files changed, 389 insertions(+)
create mode 100644 src/external/cifsidmap.m4
create mode 100644 src/lib/cifs_idmap_sss/cifs_idmap_sss.c
diff --git a/src/external/cifsidmap.m4 b/src/external/cifsidmap.m4
new file mode 100644
index
0000000000000000000000000000000000000000..546c373b0e66dd4136bca8d92cf220102b2f7830
--- /dev/null
+++ b/src/external/cifsidmap.m4
@@ -0,0 +1,13 @@
+AC_ARG_ENABLE([cifs-idmap-plugin],
+ [AS_HELP_STRING([--disable-cifs-idmap-plugin],
+ [do not build CIFS idmap plugin])],
+ [build_cifs_idmap_plugin=$enableval],
+ [build_cifs_idmap_plugin=yes])
^^^^^^
This line mean that cifs_idmap_plugin will be build by default,
but sssd.spec.in file is not updated with appropriate devel package.
As Sumit said, he'll do that part..
Something like
%if (0%{?fedora} >= 17)
BuildRequires: cifs-utils-devel
%endif
BTW: cifs-utils-devel is not available on RHEL6, do we want to build
cifs_idmap_plugin by default?
And library cifs_idmap_sss.so should be added to some (or new) subpackage
in spec file. If you are not familiar with rpm packaging, we can help you.
+
+AS_IF([test x$build_cifs_idmap_plugin = xyes],
+ [AC_CHECK_HEADER([cifsidmap.h], [],
+ [AC_MSG_ERROR([you must have the cifsidmap header
installed to build the idmap plugin])])
+ ])
+
+AM_CONDITIONAL([BUILD_CIFS_IDMAP_PLUGIN],
+ [test x$build_cifs_idmap_plugin = xyes])
diff --git a/src/lib/cifs_idmap_sss/cifs_idmap_sss.c
b/src/lib/cifs_idmap_sss/cifs_idmap_sss.c
new file mode 100644
index
0000000000000000000000000000000000000000..bcea6d6d84dbd6184c2f719d0c1b4a2ac3c014ca
--- /dev/null
+++ b/src/lib/cifs_idmap_sss/cifs_idmap_sss.c
... snip ...
+static int samba_unix_sid_to_id(const char *sid, struct cifs_uxid *cuxid)
+{
+ id_t id;
+ uint8_t type;
+
+ if (sscanf(sid, "S-1-22-%hhu-%u", &type, &id) != 2)
+ return -1;
+
+ switch (type) {
+ case 1:
+ cuxid->type = CIFS_UXID_TYPE_UID;
+ cuxid->id.uid = id;
+ break;
+ case 2:
+ cuxid->type = CIFS_UXID_TYPE_GID;
+ cuxid->id.gid = id;
^^^^^^^
Missing brake statement.
Is it aim or mistake?
That was a mistake, thanks.
+ default:
+ cuxid->type = CIFS_UXID_TYPE_UNKNOWN;
+ return -1;
+ }
+
+ return 0;
+}
And the last note.
You created new dynamic library. Could you add this library to dlopen test?
Done. New patch attached.
+++ b/src/tests/dlopen-tests.c
@@ -40,6 +40,7 @@ struct so {
{ "libsss_debug.so", { LIBPFX"libsss_debug.so", NULL } },
{ "libipa_hbac.so", { LIBPFX"libipa_hbac.so", NULL } },
{ "libsss_idmap.so", { LIBPFX"libsss_idmap.so", NULL } },
+ { "cifs_idmap_sss.so", { LIBPFX"cifs_idmap_sss.so", NULL } },
{ "libsss_nss_idmap.so", { LIBPFX"libsss_nss_idmap.so", NULL }
},
{ "libnss_sss.so", { LIBPFX"libnss_sss.so", NULL } },
{ "pam_sss.so", { LIBPFX"pam_sss.so", NULL } },
Thank you very much .
LS
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel