Message ID | 20210205112751.13559-1-lukma@denx.de |
---|---|
State | New |
Headers | show |
Series | [v3] tst: Provide test for ppoll | expand |
On 2/5/21 4:57 PM, Lukasz Majewski wrote: > This change adds new test to assess ppoll()'s timeout related > functionality (the struct pollfd does not provide valid fd to wait > for - just wait for timeout). > > To be more specific - two use cases are checked: > - if ppoll() times out immediately when passed struct timespec has zero > values of tv_nsec and tv_sec. > - if ppoll() times out after timeout specified in passed argument > > --- > Changes for v2: > - Remove _GNU_SOURCE definition if not already defined > - Replace clock_gettime with xclock_gettime > - Use FAIL_EXIT1 instead of plain ret value returning from the test > > Changes for v3: > - Use smaller timeout - 0.5s (TIMEOUT* defines can be removed) > - Use TEST_TIMESPEC_NOW_OR_AFTER () to check if test has properly timed > out Lukasz, looks like this got pushed to master without a review. Perhaps it happened by accident when pushing the timerfd patch? Siddhesh
Hi Siddhesh, > On 2/5/21 4:57 PM, Lukasz Majewski wrote: > > This change adds new test to assess ppoll()'s timeout related > > functionality (the struct pollfd does not provide valid fd to wait > > for - just wait for timeout). > > > > To be more specific - two use cases are checked: > > - if ppoll() times out immediately when passed struct timespec has > > zero values of tv_nsec and tv_sec. > > - if ppoll() times out after timeout specified in passed argument > > > > --- > > Changes for v2: > > - Remove _GNU_SOURCE definition if not already defined > > - Replace clock_gettime with xclock_gettime > > - Use FAIL_EXIT1 instead of plain ret value returning from the test > > > > Changes for v3: > > - Use smaller timeout - 0.5s (TIMEOUT* defines can be removed) > > - Use TEST_TIMESPEC_NOW_OR_AFTER () to check if test has properly > > timed out > > Lukasz, looks like this got pushed to master without a review. With this particular version (v3) - I've followed detailed hints provided by Adhemerval: https://patchwork.sourceware.org/project/glibc/patch/20210114163239.16505-1-lukma@denx.de/ > Perhaps it happened by accident when pushing the timerfd patch? I though that this patch is OK, after adding changes proposed by Adhemerval (Adhemerval in detailed way - with working code - wrote what he want's to see). (Maybe I shouldn't post v3 to ML?) > > Siddhesh Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On 2/8/21 9:36 PM, Lukasz Majewski wrote: > Hi Siddhesh, > >> On 2/5/21 4:57 PM, Lukasz Majewski wrote: >>> This change adds new test to assess ppoll()'s timeout related >>> functionality (the struct pollfd does not provide valid fd to wait >>> for - just wait for timeout). >>> >>> To be more specific - two use cases are checked: >>> - if ppoll() times out immediately when passed struct timespec has >>> zero values of tv_nsec and tv_sec. >>> - if ppoll() times out after timeout specified in passed argument >>> >>> --- >>> Changes for v2: >>> - Remove _GNU_SOURCE definition if not already defined >>> - Replace clock_gettime with xclock_gettime >>> - Use FAIL_EXIT1 instead of plain ret value returning from the test >>> >>> Changes for v3: >>> - Use smaller timeout - 0.5s (TIMEOUT* defines can be removed) >>> - Use TEST_TIMESPEC_NOW_OR_AFTER () to check if test has properly >>> timed out >> >> Lukasz, looks like this got pushed to master without a review. > > With this particular version (v3) - I've followed detailed hints > provided by Adhemerval: > https://patchwork.sourceware.org/project/glibc/patch/20210114163239.16505-1-lukma@denx.de/ > >> Perhaps it happened by accident when pushing the timerfd patch? > > I though that this patch is OK, after adding changes proposed by > Adhemerval (Adhemerval in detailed way - with working code - wrote what > he want's to see). > > (Maybe I shouldn't post v3 to ML?) Typically we commit without additional review if changes from one version to another are trivial *and* the reviewer explicitly states that. Given that the reviewer didn't explicitly say "OK to commit with nits fixed" (or similar), it's generally a good idea to wait for a review in such cases. Also, given that Adhemerval gave working code, the commit should also contain a Co-authored-by tag. Adhemerval, could you please confirm if this v3 is what you're looking for? Given that it's just a test case, I reckon a revert is not necessary and Lukasz can just fix things up if you need him to add any more fixes. Please note though that the patch that went into master is slightly different from the v3. I suspect it's just additional includes. Siddhesh
Hi Siddhesh, > On 2/8/21 9:36 PM, Lukasz Majewski wrote: > > Hi Siddhesh, > > > >> On 2/5/21 4:57 PM, Lukasz Majewski wrote: > >>> This change adds new test to assess ppoll()'s timeout related > >>> functionality (the struct pollfd does not provide valid fd to wait > >>> for - just wait for timeout). > >>> > >>> To be more specific - two use cases are checked: > >>> - if ppoll() times out immediately when passed struct timespec has > >>> zero values of tv_nsec and tv_sec. > >>> - if ppoll() times out after timeout specified in passed argument > >>> > >>> --- > >>> Changes for v2: > >>> - Remove _GNU_SOURCE definition if not already defined > >>> - Replace clock_gettime with xclock_gettime > >>> - Use FAIL_EXIT1 instead of plain ret value returning from the > >>> test > >>> > >>> Changes for v3: > >>> - Use smaller timeout - 0.5s (TIMEOUT* defines can be removed) > >>> - Use TEST_TIMESPEC_NOW_OR_AFTER () to check if test has properly > >>> timed out > >> > >> Lukasz, looks like this got pushed to master without a review. > > > > With this particular version (v3) - I've followed detailed hints > > provided by Adhemerval: > > https://patchwork.sourceware.org/project/glibc/patch/20210114163239.16505-1-lukma@denx.de/ > > > >> Perhaps it happened by accident when pushing the timerfd patch? > > > > I though that this patch is OK, after adding changes proposed by > > Adhemerval (Adhemerval in detailed way - with working code - wrote > > what he want's to see). > > > > (Maybe I shouldn't post v3 to ML?) > > Typically we commit without additional review if changes from one > version to another are trivial *and* the reviewer explicitly states > that. Given that the reviewer didn't explicitly say "OK to commit > with nits fixed" (or similar), it's generally a good idea to wait for > a review in such cases. Also, given that Adhemerval gave working > code, the commit should also contain a Co-authored-by tag. > Thanks for the explanation. I should have wait for the explicit OK from Adhemerval. > Adhemerval, could you please confirm if this v3 is what you're > looking for? Given that it's just a test case, I reckon a revert is > not necessary and Lukasz can just fix things up if you need him to > add any more fixes. I will fix them if needed. > Please note though that the patch that went into > master is slightly different from the v3. I suspect it's just > additional includes. > > Siddhesh Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index 7503b356c8..f4029a74ca 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -108,7 +108,7 @@ tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \ test-errno-linux tst-memfd_create tst-mlock2 tst-pkey \ tst-rlimit-infinity tst-ofdlocks tst-gettid tst-gettid-kill \ tst-tgkill tst-sysvsem-linux tst-sysvmsg-linux tst-sysvshm-linux \ - tst-timerfd + tst-timerfd tst-ppoll tests-internal += tst-ofdlocks-compat tst-sigcontext-get_pc CFLAGS-tst-sigcontext-get_pc.c = -fasynchronous-unwind-tables diff --git a/sysdeps/unix/sysv/linux/tst-ppoll.c b/sysdeps/unix/sysv/linux/tst-ppoll.c new file mode 100644 index 0000000000..29d92485e8 --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-ppoll.c @@ -0,0 +1,53 @@ +/* Test for ppoll timeout + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <time.h> +#include <poll.h> +#include <support/check.h> +#include <support/xtime.h> + +static int test_ppoll_timeout (bool zero_tmp) +{ + /* We wait for half a second. */ + struct timespec ts; + xclock_gettime (CLOCK_REALTIME, &ts); + struct timespec timeout = make_timespec (0, zero_tmo ? 0 : TIMESPEC_HZ/2); + ts = timespec_add (ts, timeout); + + /* Ignore fds - just wait for timeout. */ + struct pollfd fds = { -1, 0, 0 }; + TEST_COMPARE (ppoll (&fds, 1, &timeout, 0), 0); + + TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts); + + return 0; +} + +static int +do_test (void) +{ + /* Check if ppoll exits immediately. */ + test_ppoll_timeout (true); + + /* Check if ppoll exits after specified timeout. */ + test_ppoll_timeout (false); + + return 0; +} + +#include <support/test-driver.c>