New patch set is attached, I hoped it covers all the issues. Some of
them are implemented as separate patch because either is was too
complicated to do the change in the middle of pachset or I wanted the
changes to be more visible.
On 02/17/2015 03:50 PM, Jakub Hrozek wrote:
> struct tevent_req *
> +cache_req_group_by_name_send(TALLOC_CTX *mem_ctx,
> + struct tevent_context *ev,
> + struct resp_ctx *rctx,
> + struct sss_nc_ctx *ncache,
> + int neg_timeout,
> + int cache_refresh_percent,
> + const char *domain,
> + const char *name)
> +{
> + struct cache_req_input *input;
> +
> + input = cache_req_input_create(mem_ctx, CACHE_REQ_GROUP_BY_NAME, name, 0);
> + if (input == NULL) {
> + return NULL;
> + }
cache_req_input_create can fail for multiple reasons, I think we should
add a new error code instead of returning NULL. When NULL is returned
from the send function, it usually is treated as ENOMEM by the caller.
I made cache_req_input_create() to fail only on ENOMEM and move check
id==0 to cache_req_send since it is a valid query unlike name==NULL.
One additional question - are you also planning to add lookups by
UPN?
It seems that's the only functionality not implemented by this shared
lookup code.
Thank you for reminding me. The original code predates UPN changes. I
will create a patch after this huge patch set it accepted. Do you agree?
In general I like the approach you've taken but I think we can
make it
even better. Add another structure like cache_req_dom_ctx and add it inside
the cache_req_input structure. Always create cache_req_dom_ctx as a
talloc child of cache_req_input.
This would not only collapse the above talloc_frees into one, but it
would also nicely separate the per-domain data from per-lookup data and
avoid mixing data from different domains.
I played with this a little bit but it actually turned out that it makes
the code more complicated. Since you need to have access to both input
and dom_ctx anyway it doesn't really bring anything from above. I don't
have a patch ready since I abandoned it before working completion. Do
you want me to try again and finish it?
I also think dom_name sounds better than safe_name, but meh.
I went with dom_objname since "dom_name" refers to domain name.