[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