On 08/12/2016 09:17 AM, Petr Cech wrote:
> On 08/11/2016 08:53 AM, Petr Cech wrote:
>> On 08/03/2016 12:34 PM, Michal Židek wrote:
>>> Two nitpicks, see inline.
>>>
>>> On 07/22/2016 02:34 PM, Petr Cech wrote:
>>>>
>>>> +static errno_t add_to_missing_attrs (TALLOC_CTX * mem_ctx,
>>>> + struct sysdb_attrs *attrs,
>>>> + const char *ext_key,
>>>> + char ***_missing)
>>> ^
>>> Coding style. Remove the space between function name and "(".
>>> Do not forget to align the parameters after that.
>>
>> Addressed.
>>
>>>> +{
>>>> + bool is_present = false;
>>>> + size_t size = 0;
>>>> + size_t ret;
>>>> +
>>>> + for (int i = 0; i < attrs->num; i++) {
>>>> + if (strcmp(ext_key, attrs->a[i].name) == 0) {
>>>> + is_present = true;
>>>> + }
>>>> + size++;
>>>> + }
>>>> +
>>>> + if (is_present == false) {
>>>> + ret = add_string_to_list(attrs, ext_key, _missing);
>>>> + if (ret != EOK) {
>>>> + goto fail;
>>>> + }
>>>> + }
>>>> +
>>>> + ret = EOK;
>>>> +
>>>> +fail:
>>>
>>> Please change the label name to "done". According to
>>> our new coding style, the code that follows label "fail"
>>> is only executed when failure occurs. I know we do not
>>> follow this everywhere, but I would like to be consistent
>>> in new code.
>>
>> Addressed.
>>
>>>> + return ret;
>>>> +}
>>>> +
>>>
>>> Other than that it looks good to me.
>>>
>>> Also it would be good to add a CI tests for this. I do
>>> not want to postpone this patch before release, so you can
>>> do it later as part of this ticket:
>>>
https://fedorahosted.org/sssd/ticket/3119
>>>
>>> So either send a patch with CI test now or
>>> assign the above ticket to yourself and do it
>>> when there is more time.
>>>
>>> Michal
>>
>> Hello,
>>
>> there is fixed patch attached.
>>
>> I am working on CI test. I hope it will be ready soon.
>>
>> Regards.
>
> CI INTG tests will be solved in another mail thread. They are dependent
> on Lukas patches.
Tests are solved in
[SSSD] [PATCH SET] WIP: INTG: Tests for ldap nested netgroups
mainling thread.
Hello,
whole patch set for
LDAP: Fixing of removing netgroup from cache
is in
[SSSD] [PATCH SET] WIP: INTG: Tests for ldap nested netgroups
mailing thread.
--
Petr^4 Čech