[SSSD] [PRELIMINARY][PATCH] ifp users and groups

Jakub Hrozek jhrozek at redhat.com
Wed Apr 22 13:37:32 UTC 2015


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.



More information about the sssd-devel mailing list