On Tue, Jul 15, 2014 at 6:17 PM, Jakub Hrozek <jhrozek(a)redhat.com> wrote:
On Mon, Jul 14, 2014 at 10:04:50PM +0300, Noam Meltzer wrote:
> On Mon, Jul 14, 2014 at 7:25 PM, Jakub Hrozek <jhrozek(a)redhat.com>
wrote:
>
> > On Fri, Jun 27, 2014 at 09:44:35AM +0300, Noam Meltzer wrote:
> > > Implementation of design document:
> > >
https://fedorahosted.org/sssd/wiki/DesignDocs/rpc.idmapd%20plugin
> >
> > I've got just one style nitpick and one question, I can fix it before
> > pushing..
> >
> > > ---
> > > src/sss_client/nfs/sss_nfs_client.c | 571
> > ++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 571 insertions(+)
> > > create mode 100644 src/sss_client/nfs/sss_nfs_client.c
> > >
> > > diff --git a/src/sss_client/nfs/sss_nfs_client.c
> > b/src/sss_client/nfs/sss_nfs_client.c
> > > new file mode 100644
> > > index 0000000..64cb67a
> > > --- /dev/null
> > > +++ b/src/sss_client/nfs/sss_nfs_client.c
> > > @@ -0,0 +1,571 @@
> > > +/*
> > > + SSSD
> > > +
> > > + NFS Client
> > > +
> > > + Copyright (C) Noam Meltzer <noam(a)primarydata.com> 2013-2014
> > > + Copyright (C) Noam Meltzer <tsnoam(a)gmail.com> 2014-
> > > +
> > > + This program is free software; you can redistribute it and/or
modify
> > > + it under the terms of the GNU General Public License as
published by
> > > + the Free Software Foundation; either version 3 of the License, or
> > > + (at your option) any later version.
> > > +
> > > + This program is distributed in the hope that it will be useful,
> > > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + GNU General Public License for more details.
> > > +
> > > + You should have received a copy of the GNU General Public License
> > > + along with this program. If not, see <
http://www.gnu.org/licenses/
> > >.
> > > +*/
> >
> > [snip]
> >
> > > +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
. .
> > . . .*/
> > > +/* libnfsidmap return-code aids */
> > > +
> > > +/*
> > > + * we only want to return 0 or ENOENT; otherwise libnfsidmap will
stop
> > > + * translation instead of proceeding to the next translation plugin
> > > + */
> > > +int normalise_rc(int rc) {
> >
> > Style violation, I'd like the definition to read:
> > int normalise_rc(int rc)
> > {
> > }
> >
> > instead
> >
> > > + int res;
> > > +
> > > + res = rc;
> > > + if (res != 0 && res != ENOENT) {
> > > + res = ENOENT;
> > > + }
> > > +
> > > + return res;
> > > +}
> > > +
> > > +/* log the actual rc from our code (to be used before normalising
the
> > rc) */
> > > +void log_actual_rc(const char *trans_name, int rc) {
> >
> > Same style violation here. I can fix these two myself before pushing
> > provided these are the only changes we'd be doing.
> >
>
> thanks. (feeling ashamed...)
>
>
>
> >
> > > + char tmp[80];
> > > + IDMAP_LOG(1, ("%s: rc=%i msg=%s", trans_name, rc,
> > > + strerror_r(rc, tmp, sizeof(tmp))));
> > > +}
> > > +
> > > +
> > > +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
. .
> > . . .*/
> > > +/* The external interface */
> > > +static int sss_nfs_init(void)
> > > +{
> > > + nfs_use_mc = nfs_conf_get_bool(nfs_conf_sect, nfs_conf_use_mc,
> > > + USE_MC_DEFAULT);
> > > + IDMAP_LOG(1, ("%s: use memcache: %i", __func__,
nfs_use_mc));
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int sss_nfs_princ_to_ids(char *secname, char *princ, uid_t
*uid,
> > > + gid_t *gid, extra_mapping_params
**ex)
> > > +{
> > > + IDMAP_LOG(0, ("%s: not implemented", __func__));
> > > + return -ENOENT;
> >
> > Is ENOENT the return code NFS expects? Normally for functions that are
> > not implemented I'd use ENOSYS..not a big deal though, I'm mostly
> > asking..
> >
>
> for the same reason as normalise_rc() is needed: libnfsidmap will not
> proceed for the next plugin if the return code is not -ENOENT.
Thanks for the explanation, I should have read the comments more
carefully.
Here is a diff I'm going to squash in when I push the patches:
diff --git a/src/sss_client/nfs/sss_nfs_client.c
b/src/sss_client/nfs/sss_nfs_client.c
index
64cb67a8b75ec04c1d6fa03905f5427bbe6c1e82..1f1f992af60a9cde7d1f039dd329d0d52c5fe9a3
100644
--- a/src/sss_client/nfs/sss_nfs_client.c
+++ b/src/sss_client/nfs/sss_nfs_client.c
@@ -407,7 +407,8 @@ static int nfs_conf_get_bool(char *sect, char *attr,
int def)
* we only want to return 0 or ENOENT; otherwise libnfsidmap will stop
* translation instead of proceeding to the next translation plugin
*/
-int normalise_rc(int rc) {
+int normalise_rc(int rc)
+{
int res;
res = rc;
@@ -419,7 +420,8 @@ int normalise_rc(int rc) {
}
/* log the actual rc from our code (to be used before normalising the rc)
*/
-void log_actual_rc(const char *trans_name, int rc) {
+void log_actual_rc(const char *trans_name, int rc)
+{
char tmp[80];
IDMAP_LOG(1, ("%s: rc=%i msg=%s", trans_name, rc,
strerror_r(rc, tmp, sizeof(tmp))));
--
ACK