[SSSD] [PATCH] USe sysdb_serch_service() in sss_cacahe

Simo Sorce simo at redhat.com
Thu Jan 10 17:04:19 UTC 2013


On Thu, 2013-01-10 at 17:28 +0100, Jakub Hrozek wrote:
> On Thu, Jan 10, 2013 at 11:13:05AM -0500, Simo Sorce wrote:
> > On Thu, 2013-01-10 at 11:06 -0500, Simo Sorce wrote:
> > > On Thu, 2013-01-10 at 11:22 +0100, Pavel Březina wrote:
> > > > On 01/08/2013 05:10 AM, Simo Sorce wrote:
> > > > > In the patches that fix the sdap_reinit functions I introduced a new
> > > > > service search helper sysdb_search_services()
> > > > >
> > > > > Use this helper in sss_cache instead of a home grown one with poor-man
> > > > > filter parser.
> > > > >
> > > > > Simo.
> > > > 
> > > > Hi,
> > > > I don't think these functions are identical.
> > > > 
> > > > -    if (strcmp(sub_filter, "*") == 0) {
> > > > -        /* All services */
> > > > -        ret = sysdb_enumservent(mem_ctx, sysdb, &res);
> > > > -    } else {
> > > > -        /* Get service by name */
> > > > -        ret = sysdb_getservbyname(mem_ctx, sysdb, sub_filter,
> > > > -                                  NULL, &res);
> > > > -    }
> > > > 
> > > > The sub_filter is not a valid ldb filter, but '*' or name.
> > > 
> > > Ahh true, I'll change this to be a minimal wrapper then.
> > > 
> > > Thanks for catching it.
> > 
> > Actually looking at is it would be more consistent to change the filter
> > in update_all_filters, however I noticed that all these filters only to
> > SYSDB_NAME=<value> and none take in account aliases, while the
> > getsrvbyname() function also takes in account aliases. Should the
> > update_all_filters also take in account aliases for other filters like
> > update_user_filter ?
> > If so I can simply change update_filter for all uniformly.
> > 
> 
> I think it would be a good idea.
> 
> > Simo.
> > 
> > > Btw why sss_cache is not tested in make check ?
> > > 
> 
> None of the tools is. It's just noone wrote the tests upstream, we're
> relying on Red Hat QE here.

Ok I opened tickets both for alias support and tests, and another bug I
found.
I didn't do alias support because the additional bug may mean additional
changes are needed to that part of the code anyway, so I'll leave it to
someone to revisit exactly what the inputs/outputs and behavior need to
be before diving in the code.

Attached find respun patch that instead makes the code generate a proper
filter for services too so that using sysdb_serach_service() is ok.
The only thing it really changes is that it drops support for searching
service aliases, but I think that is fine as alias support is
inconsistent throughout the tool and a decision will be made in the
ticket I opened, so by dropping alias support for services we are now
more consistent even if we later decide we need to readd alias support
to all calls that support aliases.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Use-new-sysdb_search_service-in-sss_cache.patch
Type: text/x-patch
Size: 2945 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130110/394b6d2c/attachment.bin>


More information about the sssd-devel mailing list