On Fri, Oct 11, 2013 at 03:36:13PM +0200, Lukas Slebodnik wrote: > On (11/10/13 12:53), Benjamin Franzke wrote: > >2013/10/11 Sumit Bose <sb...@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 <benjaminfran...@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. > > 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.
Hi Lukas, thank you for an additional pair of eyes. Ealier in the thread I said that I'll take care of enhancing the spec file. I think cifs_idmap_plugin should be build by default and disabled on platforms like RHEL6 where cifs-utils do not uspport the plugin interface. I've planned to add the plugin to the sssd-client package. > > + > +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? good catch > > + 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? > > +++ 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@lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel