[SSSD] [PATCH] cache_req: support UPN

Pavel Březina pbrezina at redhat.com
Thu Sep 17 09:06:30 UTC 2015


On 09/17/2015 10:32 AM, Petr Cech wrote:
> Hi Pavel!
>
> There is some code between my last end and this continuation. I was read
> it and did't find anything wrong.

Hi Petr,
thank you for you review. New patches are attached. All comments from 
the previous mail should be addressed.

>> +#define run_cache_req(ctx, send_fn, done_fn, dom, crp, lookup, expret)
>> do { \
>> +    TALLOC_CTX
>> *req_mem_ctx;                                                \
>> +    struct tevent_req
>> *req;                                                 \
>> +    errno_t
>> ret;                                                            \
>> +
>> \
>> +    req_mem_ctx =
>> talloc_new(global_talloc_context);                        \
>> +
>> check_leaks_push(req_mem_ctx);                                          \
>> +
>> \
>> +    req = send_fn(req_mem_ctx, ctx->tctx->ev,
>> ctx->rctx,                    \
>> +                  ctx->ncache, 10,
>> crp,                                     \
>> +                  (dom == NULL ? NULL : dom->name),
>> lookup);                \
>> +
>> assert_non_null(req);                                                   \
>> +    tevent_req_set_callback(req, done_fn,
>> ctx);                             \
>> +
>> \
>> +    ret =
>> test_ev_loop(ctx->tctx);                                          \
>> +    assert_int_equal(ret,
>> expret);                                          \
>> +
>> assert_true(check_leaks_pop(req_mem_ctx));                              \
>> +
>> \
>> +
>> talloc_free(req_mem_ctx);                                               \
>> +} while (0)
> This definition should be a function. I found that you use it like

I wanted to avoid writing types for send_fn and done_fn. I'll do it if 
you want me to, but I think this is good enough for test.

> # return run_cache_req(...) but it doesn't provide value.

It was there originally then I realized I don't have to return a value. 
I forgot to switch errno_t with void in function definition though, thanks!

However, I did not find line "return run_cache_req...", so if it is 
there somewhere, please tell me.

>> +
>> +static void run_user_by_name(struct cache_req_test_ctx *test_ctx,
>> +                             struct sss_domain_info *domain,
>> +                             int cache_refresh_percent,
>> +                             errno_t exp_ret)
>> +{
>> +    run_cache_req(test_ctx, cache_req_user_by_name_send,
>> +                  cache_req_user_by_name_test_done, domain,
>> +                  cache_refresh_percent, TEST_USER_NAME, exp_ret);
>> +}
>> +
>> +static errno_t run_user_by_upn(struct cache_req_test_ctx *test_ctx,
>> +                               struct sss_domain_info *domain,
>> +                               int cache_refresh_percent,
>> +                               errno_t exp_ret)
>> +{
>> +    run_cache_req(test_ctx, cache_req_user_by_name_send,
>> +                  cache_req_user_by_name_test_done, domain,
>> +                  cache_refresh_percent, TEST_UPN, exp_ret);
>> +}
> This function returns errno_t but run_cache_req is
> without return statement.
>
> I tried with your patches
> # make responder_cache_req-tests
> and thre is a result:
>
> src/tests/cmocka/test_responder_cache_req.c: In function ‘run_user_by_upn’:
> src/tests/cmocka/test_responder_cache_req.c:199:1: warning: no return
> statement in function returning non-void [-Wreturn-type]
>   }
>   ^
> src/tests/cmocka/test_responder_cache_req.c: In function ‘run_user_by_id’:
> src/tests/cmocka/test_responder_cache_req.c:209:1: warning: no return
> statement in function returning non-void [-Wreturn-type]
>   }
>   ^
> src/tests/cmocka/test_responder_cache_req.c: In function
> ‘run_group_by_name’:
> src/tests/cmocka/test_responder_cache_req.c:261:1: warning: no return
> statement in function returning non-void [-Wreturn-type]
>   }
>   ^
> src/tests/cmocka/test_responder_cache_req.c: In function ‘run_group_by_id’:
> src/tests/cmocka/test_responder_cache_req.c:271:1: warning: no return
> statement in function returning non-void [-Wreturn-type]
>   }

See above.

>
> That's all for me. I didn't test the functionality. Please, ask someone
> other who has skills with AD (and running instance of it) to test it.
>
> Regards
>
> Petr
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-cache_req-provide-extra-flag-for-oob-request.patch
Type: text/x-patch
Size: 2176 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150917/847188ec/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-cache_req-add-support-for-UPN.patch
Type: text/x-patch
Size: 36335 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150917/847188ec/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-cache_req-tests-reduce-code-duplication.patch
Type: text/x-patch
Size: 83517 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150917/847188ec/attachment-0005.bin>


More information about the sssd-devel mailing list