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

Michal Židek mzidek at redhat.com
Fri Oct 19 11:07:36 UTC 2012


On 10/18/2012 09:04 PM, 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
>

I forgot to modify the test suite to use the new macro.

New patch attached.


Michal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Include-talloc-log-in-our-debug-facility.patch
Type: text/x-patch
Size: 18941 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20121019/39c146a7/attachment.bin>


More information about the sssd-devel mailing list