[SSSD] [PRELIMINARY][PATCH] ifp users and groups
Pavel Brezina
pbrezina at redhat.com
Wed Apr 22 16:46:18 UTC 2015
----- Original Message -----
> From: "Jakub Hrozek" <jhrozek at redhat.com>
> To: sssd-devel at lists.fedorahosted.org
> Sent: Wednesday, April 22, 2015 3:37:32 PM
> Subject: Re: [SSSD] [PRELIMINARY][PATCH] ifp users and groups
>
> On Wed, Apr 22, 2015 at 02:21:27PM +0200, Pavel Březina wrote:
> > Hi,
> > thanks for the comments, I'll send a fixed version when the rest is
> > finished.
> >
> > On 04/08/2015 12:48 PM, Jakub Hrozek wrote:
> > >>+
> > >>+char * ifp_users_build_path_from_msg(TALLOC_CTX *mem_ctx,
> > >>+ struct sss_domain_info *domain,
> > >>+ struct ldb_message *msg)
> > >
> > >I wonder why this function isn't static and why it's in the header?
> >
> > It's going to be used in Groups.Group interface while obtaining user
> > members
> > (probably, if not I'll remove it :-)).
>
> OK
>
> >
> > >>+errno_t ifp_users_decompose_path(struct sss_domain_info *domains,
> > >>+ const char *path,
> > >>+ struct sss_domain_info **_domain,
> > >>+ uid_t *_uid)
> > >
> > >Looks like this one could be static as well.
> >
> >
> >
> > >
> > >>+{
> > >>+ char **parts = NULL;
> > >>+ struct sss_domain_info *domain;
> > >>+ uid_t uid;
> > >>+ errno_t ret;
> > >>+
> > >>+ ret = sbus_opath_decompose_exact(NULL, path, IFP_PATH_USERS, 2,
> > >>&parts);
> > >>+ if (ret != EOK) {
> > >>+ return ret;
> > >>+ }
> > >>+
> > >>+ domain = find_domain_by_name(domains, parts[0], false);
> > >>+ if (domain == NULL) {
> > >>+ ret = ERR_DOMAIN_NOT_FOUND;
> > >>+ goto done;
> > >>+ }
> > >>+
> > >>+ uid = strtouint32(parts[1], NULL, 10);
> > >>+ if (errno != 0) {
> > >
> > >Since you use errno for error checking, you should set it to zero before
> > >calling strtouint32 (same elsewhere in this patchset).
> >
> > It is not necessary since it is set inside the function, but I can add it
> > explicitly so it is clear.
>
> Ah, then it's not necessary. I don't like functions with side-effects
> though :-)
>
> >
> > >Looking at the interface, I wonder about the members. Are they
> > >transitive or just direct members?
> >
> > This is a good question... I'm not sure from the top of my head, but I
> > think
> > we store complete membership in every user/group object, or am I wrong?
>
> Yes, I think the complete membership is a much better and in general
> more useful idea. After all, the group membership should be consistent
> with the output of getent group. If someone requires the direct members
> only we can add that in the next iteration under a different name.
Agreed. I'll mention it in the design page explicitly.
More information about the sssd-devel
mailing list