On (01/09/16 17:35), Michal Židek wrote:
> On 09/01/2016 05:26 PM, Dmitri Pal wrote:
>> Hello,
>>
>> I do not like either of the versions of the patch.
>> It is OK to use path_concat instead of snprintf. The whole point of not
>> using it was to simplify the code and not have to check yet another
>> error clause. But using path_concat is fine.
>> The thing that I do not like is that in the current code it is done in
>> both branches of the if clause.
>> I think we should move the path_concat call above the
>>
>> |/* Match names */ match = ini_aug_match_name(entry->d_name, ra_regex);|
>>
>> Logic would be:
>> - create the full path
>> - check errors
>> - call the function above
>> - use full name in both clauses as needed
>>
>> That approach IMO would be cleaner.
>>
>> --
>> Thank you,
>> Dmitri Pal
>>
>> Engineering Director, Identity Management and Platform Security
>> Red Hat, Inc.
>>
>
> Indeed. New patch is attached.
>
> Michal
>
>From 5079c97723e2865b41daa4bebe6b43e0e6685fe3 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek(a)redhat.com>
> Date: Thu, 1 Sep 2016 12:59:07 +0200
> Subject: [PATCH] ini_augment: Use full path when reporting pattern mismatch
>
> We used full path name when reporting access check
> failures but only write filename when reporting
> on pattern mismatch. This inconsistency does not
> look good when the messages are used in sssctl
> config-check.
>
> Resolves:
>
https://fedorahosted.org/sssd/ticket/3166
> ---
> ini/ini.d/merge.validator | 16 ++++++++--------
> ini/ini_augment.c | 15 ++++++++++-----
> 2 files changed, 18 insertions(+), 13 deletions(-)
>
ACK
LS