[SSSD] [PATCH] server-tests: do not use complex condition in assert

Jakub Hrozek jhrozek at redhat.com
Tue Jan 13 11:37:33 UTC 2015


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 at 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?



More information about the sssd-devel mailing list