On 01/13/2015 12:48 PM, Lukas Slebodnik wrote:
On (13/01/15 12:37), Jakub Hrozek wrote:
> On Tue, Jan 13, 2015 at 12:28:22PM +0100, Lukas Slebodnik wrote:
>> On (13/01/15 11:14), Jakub Hrozek wrote:
>>> On Fri, Jan 09, 2015 at 05:32:41PM +0100, Lukas Slebodnik wrote:
>>>> On (09/01/15 15:18), Pavel Březina wrote:
>>>>> On 01/09/2015 03:17 PM, Pavel Březina wrote:
>>>>>> If OR is used in assert macro we never know what part of the
condition
>>>>>> was not met from a failure message directly.
>>>>>>
>>>>>> I'm seeing a failure in server-tests, because pid number of
sssd process
>>>>>> is greater than 0xffff, it is bigger than 100k. Where does this
constant
>>>>>> come from?
>>>> cat /proc/sys/kernel/pid_max
>>>>
>>>> On my system i is 0x8000
>>>> echo "obase=16; `cat /proc/sys/kernel/pid_max`" | bc
>>>>
>>>>>>
>>>>>> [ RUN ] test_run_as_root_daemon
>>>>>> tmp > 0xFFFF
>>>>>> /home/pbrezina/workspace/sssd/src/tests/cwrap/test_server.c:79:
error:
>>>>>> Failure
>>>>>
>>>>> And now with the patch.
>>>>>
>>>>
>>>> >From 7d5912f3206205dc24537f3640e9a8701995ceaf Mon Sep 17 00:00:00
2001
>>>>> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
>>>>> Date: Fri, 9 Jan 2015 15:12:31 +0100
>>>>> Subject: [PATCH] server-tests: do not use complex condition in
assert
>>>>>
>>>>> If OR is used in assert macro we never know what part of the
>>>>> condition was not met from a failure message directly.
>>>>> ---
>>>>> src/tests/cwrap/test_server.c | 4 +++-
>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/src/tests/cwrap/test_server.c
b/src/tests/cwrap/test_server.c
>>>>> index
d0aeac47d0b067fdbc3399037c0a74f150337a23..560d73330f8caa5ba9706d13bae296594bb2841f 100644
>>>>> --- a/src/tests/cwrap/test_server.c
>>>>> +++ b/src/tests/cwrap/test_server.c
>>>>> @@ -75,7 +75,9 @@ static void wait_for_bg_server(const char
*pidfile)
>>>>> buf[sizeof(buf) - 1] = '\0';
>>>>>
>>>>> tmp = strtol(buf, NULL, 10);
>>>>> - assert_false(tmp == 0 || tmp > 0xFFFF || errno == ERANGE);
>>>>> + assert_false(tmp == 0);
>>>>> + assert_false(tmp > 0xFFFF);
>>>> ^^^^^^
>>>> I think we should not rely on such constant
>>>> because you can set the value higher
>>>> (up to 2^22 on 32-bit machines: 4,194,304) with:
>>>> echo 4194303 > /proc/sys/kernel/pid_max
>>>>
>>>> LS
>>>
>>> git blame points to me and IIRC I took part of these tests from
>>> uid_wrapper code without too much thinking about them.
>>>
>>> I think we can just remove the 0xFFFF part completely.
>> I agree.
>>
>> or we can use our function strtouint32 to convert number instead of strtol.
>> There is assumption sizeof(pid_t) == sizeof(uint32_t)
>
> Good idea, can either you or Pavel B. prepare a patch?
I let Pavel to finish his work :-)
LS
Attached.