On Fri, Aug 19, 2016 at 12:54:07PM +0200, Petr Cech wrote:
> On 08/17/2016 02:37 PM, Petr Cech wrote:
>> 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.
Can you change the thread subject then with the next response there? It's
a bit confusing that a fix is in WIP thread whose subject is tests..
Right, subject changed from
[SSSD] [PATCH SET] WIP: INTG: Tests for ldap nested netgroups
to
[SSSD] [PATCH] LDAP: Fixing of removing netgroup from cache
--
Petr^4 Čech