[SSSD] [PATCH] TESTS: fix segfault in test_sss_strerror_err_last

Lukas Slebodnik lslebodn at redhat.com
Fri May 29 10:01:20 UTC 2015


On (29/05/15 11:17), Pavel Reichl wrote:
>
>
>On 05/28/2015 06:52 PM, Sumit Bose wrote:
>>On Thu, May 28, 2015 at 06:26:43PM +0200, Pavel Reichl wrote:
>>>
>>>On 05/28/2015 06:12 PM, Sumit Bose wrote:
>>>>On Thu, May 28, 2015 at 05:57:35PM +0200, Lukas Slebodnik wrote:
>>>>>On (28/05/15 17:53), Pavel Reichl wrote:
>>>>>>On 05/28/2015 05:45 PM, Lukas Slebodnik wrote:
>>>>>>>On (28/05/15 17:25), Pavel Reichl wrote:
>>>>>>>>Hello,
>>>>>>>>
>>>>>>>>please see simple attached patch. Although test catches the bug and is no
>>>>>>>>doubt useful I believe that we should avoid segfaulting even in tests.
>>>>>>>>
>>>>>>>>Thanks for considering!
>>>>>>>>From a509c49fbc55664170d96e0e29bf6263c8d38a2a Mon Sep 17 00:00:00 2001
>>>>>>>>From: Pavel Reichl <preichl at redhat.com>
>>>>>>>>Date: Thu, 28 May 2015 11:13:47 -0400
>>>>>>>>Subject: [PATCH] TESTS: fix segfault in test_sss_strerror_err_last
>>>>>>>>
>>>>>>>>If there were more error codes than error messages. This
>>>>>>>>test segfaulted.
>>>>>>>>
>>>>>>>When test crashed then it failed.
>>>>>>I don't think that relying on undefined behavior is a good practice.
>>>>>It's not undefined behaviour.
>>>>>becuase you should not dereference NULL pointer.
>>>>>
>>>>>>>So the problem was not in test but in sssd code.
>>>>>>Well technically the test is part of SSSD code suite IMO.
>>>>>Test can crash if the bug is in the code.
>>>>>result of make check is non zero -> failed.
>>>>I agree with Lukas here. The crash is not in the test. but in the code.
>>>>If I see it correctly from you patch sss_strerror() crashes if the error
>>>>code is valid but there is no matching error message, because the array
>>>>is too short. Instead of doing the check in the test the check should be
>>>>done in sss_strerror() and if there is a mismatch a special error
>>>>should be return. The test then checks for proper behaviour and fails id
>>>>the special error is returned indication a mismatch in error codes and
>>>>error messages.
>>>>
>>>>HTH
>>>>
>>>>bye,
>>>>Sumit
>>>Sumit, thanks for comment. But I'm little confused here, do you agree with
>>>Lukas that this test is implemented just fine or do you agree with me that
>>I think the test is implemented fine, it checks if the last error
>>message is the expected message. If the test crashes, it is not the
>>fault of the test but the tested call, sss_strerror() has an issue.
>Thanks for explaining. I'll probably send the patch on sss_strerror() then.
>>
>>>segfault is not good test output?
>>I think it is neither good or bad, it just indicates that the test
>>discovered an error.
>>
>>bye,
>>Sumit
>OK, apparently I'm on the wrong side of this discussion. I yield, I just want
>to learn from my mistakes, so can you please tell me when I was wrong:
>
>a) Dereferencing NULL pointer is from C language pov undefined behavior and
>virtually anything can happen => test might pass.
Even thought there is a VERY VERY VERY VERY VERY little change
that test will pass in normal mode due to undefined behaviour.
It will not pass with valgrind => CI would fail.

LS



More information about the sssd-devel mailing list