[SSSD] [PATCH] util: Include disabled domains in link_forest_roots

Michal Židek mzidek at redhat.com
Tue Sep 15 13:53:41 UTC 2015


On 09/14/2015 05:43 PM, Jakub Hrozek wrote:
> On Wed, Sep 09, 2015 at 02:43:59PM +0200, Michal Židek wrote:
>> Hi,
>>
>> patch for ticket
>> https://fedorahosted.org/sssd/ticket/2673
>> is in the attachment.
>>
>> Thanks.
>> Michal
>
>>  From 7c454bc2a737be05068418a5eef7fe9446bb5fa8 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek at redhat.com>
>> Date: Wed, 9 Sep 2015 14:37:48 +0200
>> Subject: [PATCH] util: Include disabled domains in link_forest_roots
>>
>> Ticket:
>> https://fedorahosted.org/sssd/ticket/2673
>> ---
>>   src/db/sysdb_subdomains.c     |  6 +++---
>>   src/tests/cmocka/test_utils.c |  3 +++
>>   src/util/domain_info_utils.c  | 21 ++++++++++++++++++---
>>   src/util/util.h               |  3 +++
>>   4 files changed, 27 insertions(+), 6 deletions(-)
>
> The patch looks good but whenever I see us adding more and more boolean
> switches, I wonder if we should just use flags instead?
>
> This:
>      get_next_domain_ex(d, GND_USE_DISABLED | GND_DESCEND);
> Reads quite a bit easier to me than:
>      get_next_domain_ex(d, true, false);
>
> Also, bonus point are acquired next time we add a new flag, because not
> all callers of the function must be converted..
>
> What do you think?

I think this is very good point. It will also give us implicit
boolean parameter value (if not set, it is automatically false)
which improves readability a lot. I wander if it is good to add
the _ex function in this case. Would you agree if I changed the
original get_next_domain to use flags? I know, it needs to be changed
on more places, but I think such small refactoring makes more
sence than adding _ex function with flags.

Or is this what you were saying? I am not sure because
you used the _ex function in your example of "nice"
usage.

Michal



More information about the sssd-devel mailing list