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 <jhro...@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.
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org