<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jul 14, 2014 at 7:25 PM, Jakub Hrozek <span dir="ltr">&lt;<a href="mailto:jhrozek@redhat.com" target="_blank">jhrozek@redhat.com</a>&gt;</span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">On Fri, Jun 27, 2014 at 09:44:35AM +0300, Noam Meltzer wrote:<br>

&gt; Implementation of design document:<br>
&gt; <a href="https://fedorahosted.org/sssd/wiki/DesignDocs/rpc.idmapd%20plugin" target="_blank">https://fedorahosted.org/sssd/wiki/DesignDocs/rpc.idmapd%20plugin</a><br>
<br>
</div>I&#39;ve got just one style nitpick and one question, I can fix it before pushing..<br>
<div><div class="h5"><br>
&gt; ---<br>
&gt;  src/sss_client/nfs/sss_nfs_client.c | 571 ++++++++++++++++++++++++++++++++++++<br>
&gt;  1 file changed, 571 insertions(+)<br>
&gt;  create mode 100644 src/sss_client/nfs/sss_nfs_client.c<br>
&gt;<br>
&gt; diff --git a/src/sss_client/nfs/sss_nfs_client.c b/src/sss_client/nfs/sss_nfs_client.c<br>
&gt; new file mode 100644<br>
&gt; index 0000000..64cb67a<br>
&gt; --- /dev/null<br>
&gt; +++ b/src/sss_client/nfs/sss_nfs_client.c<br>
&gt; @@ -0,0 +1,571 @@<br>
&gt; +/*<br>
&gt; +   SSSD<br>
&gt; +<br>
&gt; +   NFS Client<br>
&gt; +<br>
&gt; +   Copyright (C) Noam Meltzer &lt;<a href="mailto:noam@primarydata.com">noam@primarydata.com</a>&gt;    2013-2014<br>
&gt; +   Copyright (C) Noam Meltzer &lt;<a href="mailto:tsnoam@gmail.com">tsnoam@gmail.com</a>&gt;        2014-<br>
&gt; +<br>
&gt; +   This program is free software; you can redistribute it and/or modify<br>
&gt; +   it under the terms of the GNU General Public License as published by<br>
&gt; +   the Free Software Foundation; either version 3 of the License, or<br>
&gt; +   (at your option) any later version.<br>
&gt; +<br>
&gt; +   This program is distributed in the hope that it will be useful,<br>
&gt; +   but WITHOUT ANY WARRANTY; without even the implied warranty of<br>
&gt; +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the<br>
&gt; +   GNU General Public License for more details.<br>
&gt; +<br>
&gt; +   You should have received a copy of the GNU General Public License<br>
&gt; +   along with this program.  If not, see &lt;<a href="http://www.gnu.org/licenses/" target="_blank">http://www.gnu.org/licenses/</a>&gt;.<br>
&gt; +*/<br>
<br>
</div></div>[snip]<br>
<div class=""><br>
&gt; +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/<br>
&gt; +/* libnfsidmap return-code aids */<br>
&gt; +<br>
&gt; +/*<br>
&gt; + * we only want to return 0 or ENOENT; otherwise libnfsidmap will stop<br>
&gt; + * translation instead of proceeding to the next translation plugin<br>
&gt; + */<br>
&gt; +int normalise_rc(int rc) {<br>
<br>
</div>Style violation, I&#39;d like the definition to read:<br>
int normalise_rc(int rc)<br>
{<br>
}<br>
<br>
instead<br>
<div class=""><br>
&gt; +    int res;<br>
&gt; +<br>
&gt; +    res = rc;<br>
&gt; +    if (res != 0 &amp;&amp; res != ENOENT) {<br>
&gt; +        res = ENOENT;<br>
&gt; +    }<br>
&gt; +<br>
&gt; +    return res;<br>
&gt; +}<br>
&gt; +<br>
&gt; +/* log the actual rc from our code (to be used before normalising the rc) */<br>
&gt; +void log_actual_rc(const char *trans_name, int rc) {<br>
<br>
</div>Same style violation here. I can fix these two myself before pushing<br>
provided these are the only changes we&#39;d be doing.<br></blockquote><div><br></div><div>thanks. (feeling ashamed...)</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<div class=""><br>
&gt; +    char tmp[80];<br>
&gt; +    IDMAP_LOG(1, (&quot;%s: rc=%i msg=%s&quot;, trans_name, rc,<br>
&gt; +                  strerror_r(rc, tmp, sizeof(tmp))));<br>
&gt; +}<br>
&gt; +<br>
&gt; +<br>
&gt; +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/<br>
&gt; +/* The external interface */<br>
&gt; +static int sss_nfs_init(void)<br>
&gt; +{<br>
&gt; +    nfs_use_mc = nfs_conf_get_bool(nfs_conf_sect, nfs_conf_use_mc,<br>
&gt; +                                   USE_MC_DEFAULT);<br>
&gt; +    IDMAP_LOG(1, (&quot;%s: use memcache: %i&quot;, __func__, nfs_use_mc));<br>
&gt; +<br>
&gt; +    return 0;<br>
&gt; +}<br>
&gt; +<br>
&gt; +static int sss_nfs_princ_to_ids(char *secname, char *princ, uid_t *uid,<br>
&gt; +                                gid_t *gid, extra_mapping_params **ex)<br>
&gt; +{<br>
&gt; +    IDMAP_LOG(0, (&quot;%s: not implemented&quot;, __func__));<br>
&gt; +    return -ENOENT;<br>
<br>
</div>Is ENOENT the return code NFS expects? Normally for functions that are<br>
not implemented I&#39;d use ENOSYS..not a big deal though, I&#39;m mostly<br>
asking..<br></blockquote><div><br></div><div>for the same reason as normalise_rc() is needed: libnfsidmap will not proceed for the next plugin if the return code is not -ENOENT.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<br>
The rest is an ACK from me.<br>
_______________________________________________<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>
</blockquote></div><br></div></div>