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@primarydata.com>    2013-2014
> +   Copyright (C) Noam Meltzer <tsnoam@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.

> +    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..

The rest is an ACK from me.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel