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.
Regards
--
Petr^4 Čech