On Wed, Jul 09, 2014 at 09:41:08AM +0200, Pavel Reichl wrote:
> On Tue, 2014-07-08 at 10:23 +0200, Pavel Březina wrote:
>> On 07/07/2014 02:14 PM, Pavel Reichl wrote:
>>> On Wed, 2014-07-02 at 13:10 +0200, Pavel Březina wrote:
>>>> This fixes a bug found by Jan Šafránek. The debug level was not restored
>>>> after sssd restart because the time of last modification of sssd.conf
>>>> was older than config.ldb.
>>>> _______________________________________________
>>>> sssd-devel mailing list
>>>> sssd-devel(a)lists.fedorahosted.org
>>>>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>
>>> Hi Pavel,
>>>
>>>
>>>> ret = change_debug_level_tmp(ctx->rctx->cdb, name, type,
arg_new_level);
>>>> + if (ret != EOK) {
>>>> + goto done;
>>>> + }
>>>> +
>>>> + errno = 0;
>>>
>>> I just wonder if there is any need to initialize errno as it is set by
utime() in case of failure and we directly check return value of utime().
>>> I'm not sure but I don't think it's a common practice in SSSD
codebase to always initialize errno.
>>
>> You are correct sir. I removed it also from sss_debuglevel tool.
>>
>>>
>>> I think it would be better to replace this line with comment like you put in
commit message, something like
>>>
>>> /* Touch configuration file to make sure debug level is reloaded. */
>>>
>>
>> Done.
>>
>>>> + if (utime(CONFDB_DEFAULT_CONFIG_FILE, NULL) == -1) {
>>>> + ret = errno;
>>>> + goto done;
>>>> + }
>>>> +
>>>> + ret = EOK;
>>>
>>> Thanks,
>>>
>>> PR
>>>
>>>
>>> _______________________________________________
>>> sssd-devel mailing list
>>> sssd-devel(a)lists.fedorahosted.org
>>>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>
>>
>> Patches are attached.
>>
>> _______________________________________________
>> sssd-devel mailing list
>> sssd-devel(a)lists.fedorahosted.org
>>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> Hi, thanks for the update.
>
> Patches LGTM and fixes the issue.
>
> ACK to both.
I pushed the first one so far, do you think the extra errno setting is
worth a patch? If so, I will push it, too..
master: 09641f85aae8fff72d0573394c40c8c3c002179a
>
> Regards,
>
> PR
>
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel