On Mon, May 20, 2013 at 09:41:02AM +0200, Lukas Slebodnik wrote:
On (18/05/13 18:55), Jakub Hrozek wrote:
>On Fri, May 17, 2013 at 07:29:01PM +0200, Lukas Slebodnik wrote:
>> On (17/05/13 18:50), Lukas Slebodnik wrote:
>> >ehlo,
>> >
>> >This week Ondra fix segfault, which was caused by missing argument
>> >in debug message.
>> >> ("nsupdate_get_addrs_done failed: [%d]: [%s]\n",
>> >> sss_strerror(ret)));
>> >
>> >I was working on another ticket and I spotted similar bug.
>> >So I decided to enable printf format checking in debug_fn.
>> >There was a huge number of warnings. Attached patch fixes the most
critical.
>> >--missing argument(s).
>> >--format '%s', but argument is integer.
>> >--wrong format string, example: '%\n'
>> >
>> >There are another 150 warnings, but they aren't critical.
>> >for example:
>> >format '%d' expects argument of type 'int', but argument 2
has type 'size_t'
>> >
>
>Hi,
>
>this is a really good patch that fixes many potentially bad issues.
>
>I think we should still fix these eventually, for instance for size_t,
>we should use the %z conversion.
>
>> I forgot to wrote in the first mail:
>> Remaining warnings are not critical, but they are. So I don't want to
pollute
>> output from compiler with warnings, therefore patch did not enable
>> printf format checking in debug_fn.
>>
>
>What if we enabled the warnings only if SSSD was compiled with a certain
>preprocessor directive? Then the developers would see if they introduce
>any new warning, but compiling the "production" code that runs with
>debug_level=0 anyway would not produce any warnings.
>
I think we should fix all warnings, because it is hard to find new introduced
warning among another 150 "integer format warnings"
(int -> {size_t, uint32_t, long ...})
Should I file a ticket?
>About the patch - almost ack. Please squash the attached patch to fix
>some debug levels and I'll ack.
Thank you. Patch squashed.
I squased another patch to improve readability.
Ack