[SSSD] [PATCH] Include talloc log in our debug facility

Jakub Hrozek jhrozek at redhat.com
Fri Oct 19 11:00:12 UTC 2012


On Thu, Oct 18, 2012 at 09:04:26PM +0200, Michal Židek wrote:
> On 10/15/2012 05:12 PM, Pavel Březina wrote:
> >On 10/15/2012 03:34 PM, Michal Židek wrote:
> >>This patch introduces  use_talloc_logs function. After this function is
> >>called, talloc log messages are sended to DEBUG macro and shown in our
> >>logs.
> >>
> >>https://fedorahosted.org/sssd/ticket/1495
> >>
> >>Patch is attached.
> >>
> >>Michal
> >
> >Hi,
> >I didn't do a comprehensive review yet, but I don't really like the way
> >you are using use_talloc_log(). I think it would be much better to
> >create a function (say debug_init()) that would take care of debug
> >initialization i.e. setting debug level and enabling talloc. It should
> >basically replace debug_convert_old_level() and
> >CONVERT_AND_SET_DEBUG_LEVEL() calls.
> 
> Ok. I replaced the debug_convert_old_level() and
> CONVERT_AND_SET_DEBUG_LEVEL() calls with new DEBUG_INIT() macro,
> which also sets the talloc_log_fn properly.
> 
> >
> >There are quite a few places where we are initializing our debug
> >facility so this would make it easier to maintain in the future.
> >
> 
> I was also thinking about setting the debug_prg_name inside of this
> macro too. At first I thought it is a good idea, but now I don't
> think so, because we would still need to create the string (prog.
> name) outside this macro. Now we do it in one operation with the
> assignment to debug_prg_name so we would actually need to do more
> than we do now. As it is now, it looks more consistent to me.
> 
> >The talloc logging itself looks al right, but can you prepend "talloc: "
> >before the message?
> 
> This is not needed. The messages are printed from talloc_log_fn
> function so they already look like this:
> 
> (Thu Oct 18 20:12:21:494900 2012) [sss_debuglevel] [talloc_log_fn]
> (0x0010): talloc: access after free error - first free may be at
> src/tools/sss_debuglevel.c:94
> (Thu Oct 18 20:12:21:495177 2012) [sss_debuglevel] [talloc_log_fn]
> (0x0010): Bad talloc magic value - access after free
> 
> 
> New patch is in attachment.
> 
> Thanks
> Michal

You also need to fix the unit tests:
src/tests/strtonum-tests.c: In function ‘main’:
src/tests/strtonum-tests.c:452:5: error: implicit declaration of
function ‘CONVERT_AND_SET_DEBUG_LEVEL’
[-Werror=implicit-function-declaration]
cc1: some warnings being treated as errors
make[2]: *** [src/tests/strtonum_tests-strtonum-tests.o] Error 1
make[2]: *** Waiting for unfinished jobs....
src/tests/sysdb-tests.c: In function ‘main’:
src/tests/sysdb-tests.c:4102:5: error: implicit declaration of function
‘CONVERT_AND_SET_DEBUG_LEVEL’ [-Werror=implicit-function-declaration]



More information about the sssd-devel mailing list