[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