[SSSD] [PATCH] cache_req: support UPN
Pavel Březina
pbrezina at redhat.com
Tue Oct 6 13:16:15 UTC 2015
On 10/01/2015 05:06 PM, Sumit Bose wrote:
> On Thu, Sep 17, 2015 at 11:06:30AM +0200, Pavel Březina wrote:
>> 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.
>>
>
>
> [PATCH 2/3] cache_req: add support for UPN
>
>> diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
>> index 5f33b225a31e1c1add7f776215d500fb09e127ca..a72a8bac594dc10ff9aa337399f27d9936830a8e 100644
>> --- a/src/db/sysdb_search.c
>> +++ b/src/db/sysdb_search.c
>> @@ -295,6 +295,59 @@ static char *enum_filter(TALLOC_CTX *mem_ctx,
>> return filter;
>> }
>>
>> +int sysdb_getpwupn(TALLOC_CTX *mem_ctx,
>> + struct sss_domain_info *domain,
>> + const char *upn,
>> + struct ldb_result **_res)
>> +{
>> + TALLOC_CTX *tmp_ctx;
>> + struct ldb_message *msg;
>> + struct ldb_result *res;
>> + static const char *attrs[] = SYSDB_PW_ATTRS;
>> + errno_t ret;
>> +
>> + tmp_ctx = talloc_new(NULL);
>> + if (tmp_ctx == NULL) {
>> + DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
>> + return ENOMEM;
>> + }
>> +
>> + ret = sysdb_search_user_by_upn(tmp_ctx, domain, upn, attrs, &msg);
>> + if (ret != EOK && ret != ENOENT) {
>> + DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn() failed.\n");
>> + goto done;
>> + }
>
> The call stack here would be
>
> sysdb_getpwupn()->sysdb_search_user_by_upn()->sysdb_search_entry()->ldb_search()
>
> and the returned results, starting from ldb_search()
>
> ldb_result -> ldb_message (array) -> ldb_message -> ldb_result
>
> I think it would be easier and safe some allocations if it would be
>
> sysdb_search_user_by_upn()->sysdb_getpwupn()->ldb_search() .
>
> Do you think this change makes sense?
Hi, thanks for the review. Done.
Although I created a new function sysdb_search_user_by_upn_res since you
can't pass attrs into sysdb_getpwnupn (unless we are inconsistent with api).
>
const char *orig_name;
>
> Instead of adding a new member which again will hold the originally
> provided name I would suggest to refactor the change added by
> e87b2a6e94c1066b3044fe683825ff5b4f8716c2 (cache_req: parse input name if
> needed) by introducing a member called e.g. parsed_name which keeps the
> name returned by sss_parse_inp_recv() and return it in cache_req_recv().
>
> In e87b2a6e94c1066b3044fe683825ff5b4f8716c2 it made sense to overwrite
> orig_name because no additional member was needed. But now I think the
> code would be more clear if orig_name will be kept unmodified.
Done in separate patch so the changes are more visible. It can be
squashed to the second patch.
>
>
> bye,
> Sumit
> _______________________________________________
> 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: 2178 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151006/be70263b/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-cache_req-add-support-for-UPN.patch
Type: text/x-patch
Size: 40020 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151006/be70263b/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-cache_req-tests-reduce-code-duplication.patch
Type: text/x-patch
Size: 83519 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151006/be70263b/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-cache_req-remove-raw_name-and-do-not-touch-orig_name.patch
Type: text/x-patch
Size: 4405 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151006/be70263b/attachment-0007.bin>
More information about the sssd-devel
mailing list