On 03/08/2016 10:57 PM, Lukas Slebodnik wrote:
On (08/03/16 18:14), Jakub Hrozek wrote:
> On Tue, Mar 08, 2016 at 09:34:29AM +0100, Lukas Slebodnik wrote:
>> On (25/02/16 11:06), Jakub Hrozek wrote:
>>> On Wed, Feb 24, 2016 at 06:05:11PM +0100, Lukas Slebodnik wrote:
>>>> On (24/02/16 16:43), Jakub Hrozek wrote:
>>>>> We don't know the group name at that point yet, so better not
print
>>>>> "null" in the debug message..
>>>>
>>>> >From ffdc00755a9fbaeb54f781956a0025719e532b11 Mon Sep 17 00:00:00
2001
>>>>> From: Jakub Hrozek <jhrozek(a)redhat.com>
>>>>> Date: Tue, 26 Jan 2016 16:29:08 +0100
>>>>> Subject: [PATCH] LDAP: Do not print "null" in the DEBUG
message
>>>>>
>>>>> ---
>>>>> src/providers/ldap/sdap_async_groups.c | 3 +--
>>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/src/providers/ldap/sdap_async_groups.c
b/src/providers/ldap/sdap_async_groups.c
>>>>> index
5bb267fa5331c73cb6b9b86ab21f25fcd3b0df4f..b972863a17e543361c5544382cf8ebbdde91672c 100644
>>>>> --- a/src/providers/ldap/sdap_async_groups.c
>>>>> +++ b/src/providers/ldap/sdap_async_groups.c
>>>>> @@ -538,8 +538,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
>>>>> goto done;
>>>>> }
>>>>> } else if (ret == ENOENT) {
>>>>> - DEBUG(SSSDBG_TRACE_ALL, "objectSID: not available for
group [%s].\n",
>>>>> - group_name);
>>>>> + DEBUG(SSSDBG_TRACE_ALL, "objectSID: not available for
group.\n");
>>>>> sid_str = NULL;
>>>>> } else {
>>>>> DEBUG(SSSDBG_MINOR_FAILURE, "Could not identify
objectSID: [%s]\n",
>>>>
>>>>
>>>> Could we move " sdap_get_group_primary_name(...,
&group_name)"
>>>> before "sdap_attrs_get_sid_str"?
>>>
>>> No, because we need to know the object domain first to format the name
>>> properly and in order to find the correct domain, we need to know the
>>> SID. See 970c5afb Maybe we should add a comment to that function, too,
>>> so that someone doesn't try to 'optimize' it in future..
>> Ahh, I missed that we might need different domain for
>> sdap_get_group_primary_name.
>>
>> But there is a question. Do we really need this trace debug message?
>> IMHO it's confusing debug message with ldap provider.
>
> Dunno, we can remove it as well. I'm fine either way, but printing NULL
> is potentionally dangerous.
Then I vote for removing it. I cannot see a benfit of just logging
message "objectSID: not available for group". It does not say which group
and therefore it's confusing.
I'm fine either way.