<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"><<a href="mailto:jhrozek@redhat.com" target="_blank">jhrozek@redhat.com</a>></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>
> Implementation of design document:<br>
> <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've got just one style nitpick and one question, I can fix it before pushing..<br>
<div><div class="h5"><br>
> ---<br>
> src/sss_client/nfs/sss_nfs_client.c | 571 ++++++++++++++++++++++++++++++++++++<br>
> 1 file changed, 571 insertions(+)<br>
> create mode 100644 src/sss_client/nfs/sss_nfs_client.c<br>
><br>
> diff --git a/src/sss_client/nfs/sss_nfs_client.c b/src/sss_client/nfs/sss_nfs_client.c<br>
> new file mode 100644<br>
> index 0000000..64cb67a<br>
> --- /dev/null<br>
> +++ b/src/sss_client/nfs/sss_nfs_client.c<br>
> @@ -0,0 +1,571 @@<br>
> +/*<br>
> + SSSD<br>
> +<br>
> + NFS Client<br>
> +<br>
> + Copyright (C) Noam Meltzer <<a href="mailto:noam@primarydata.com">noam@primarydata.com</a>> 2013-2014<br>
> + Copyright (C) Noam Meltzer <<a href="mailto:tsnoam@gmail.com">tsnoam@gmail.com</a>> 2014-<br>
> +<br>
> + This program is free software; you can redistribute it and/or modify<br>
> + it under the terms of the GNU General Public License as published by<br>
> + the Free Software Foundation; either version 3 of the License, or<br>
> + (at your option) any later version.<br>
> +<br>
> + This program is distributed in the hope that it will be useful,<br>
> + but WITHOUT ANY WARRANTY; without even the implied warranty of<br>
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the<br>
> + GNU General Public License for more details.<br>
> +<br>
> + You should have received a copy of the GNU General Public License<br>
> + along with this program. If not, see <<a href="http://www.gnu.org/licenses/" target="_blank">http://www.gnu.org/licenses/</a>>.<br>
> +*/<br>
<br>
</div></div>[snip]<br>
<div class=""><br>
> +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/<br>
> +/* libnfsidmap return-code aids */<br>
> +<br>
> +/*<br>
> + * we only want to return 0 or ENOENT; otherwise libnfsidmap will stop<br>
> + * translation instead of proceeding to the next translation plugin<br>
> + */<br>
> +int normalise_rc(int rc) {<br>
<br>
</div>Style violation, I'd like the definition to read:<br>
int normalise_rc(int rc)<br>
{<br>
}<br>
<br>
instead<br>
<div class=""><br>
> + int res;<br>
> +<br>
> + res = rc;<br>
> + if (res != 0 && res != ENOENT) {<br>
> + res = ENOENT;<br>
> + }<br>
> +<br>
> + return res;<br>
> +}<br>
> +<br>
> +/* log the actual rc from our code (to be used before normalising the rc) */<br>
> +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'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>
> + char tmp[80];<br>
> + IDMAP_LOG(1, ("%s: rc=%i msg=%s", trans_name, rc,<br>
> + strerror_r(rc, tmp, sizeof(tmp))));<br>
> +}<br>
> +<br>
> +<br>
> +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/<br>
> +/* The external interface */<br>
> +static int sss_nfs_init(void)<br>
> +{<br>
> + nfs_use_mc = nfs_conf_get_bool(nfs_conf_sect, nfs_conf_use_mc,<br>
> + USE_MC_DEFAULT);<br>
> + IDMAP_LOG(1, ("%s: use memcache: %i", __func__, nfs_use_mc));<br>
> +<br>
> + return 0;<br>
> +}<br>
> +<br>
> +static int sss_nfs_princ_to_ids(char *secname, char *princ, uid_t *uid,<br>
> + gid_t *gid, extra_mapping_params **ex)<br>
> +{<br>
> + IDMAP_LOG(0, ("%s: not implemented", __func__));<br>
> + return -ENOENT;<br>
<br>
</div>Is ENOENT the return code NFS expects? Normally for functions that are<br>
not implemented I'd use ENOSYS..not a big deal though, I'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>