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 <[email protected]> >>>>>>>>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 _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
