| Message ID | 20200305151459.30341-1-mdoucha@suse.cz |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v2,1/2] Add TST_ASSERT_SYSCALL*() macros | expand |
Hi Martin, > These macros take care of the standard return value checking boilerplate > in cases where the test cannot continue after error. > - TST_ASSERT_SYSCALL() calls tst_brk() if retval != 0 > - TST_ASSERT_SYSCALL_FD() calls tst_brk() if retval < 0 Reviewed-by: Petr Vorel <pvorel@suse.cz> What I like on these macros (besides DRY) is that it really shows the test, not the library, see before: tst_safe_timerfd.c:21: BROK: timerfd01.c:89 timerfd_create(CLOCK_MONOTONIC) failed: EINVAL (22) after: timerfd01.c:89: BROK: timerfd_create(CLOCK_MONOTONIC) failed: EINVAL (22) > +/* assert that syscall returned only 0 and nothing else */ > +#define TST_ASSERT_SYSCALL(SCALL) \ > + TST_ASSERT_SYSCALL_IMPL(SCALL, __FILE__, __LINE__, #SCALL) > + > +#define TST_ASSERT_SYSCALL_IMPL(SCALL, FILENAME, LINENO, CALLSTR, ...) \ > + ({ \ > + int _tst_ret; \ > + errno = 0; \ > + _tst_ret = SCALL; \ > + if (_tst_ret == -1) { \ > + int _tst_ttype = errno == ENOTSUP ? TCONF : TBROK; \ > + tst_brk_(FILENAME, LINENO, _tst_ttype | TERRNO, \ > + CALLSTR " failed", ##__VA_ARGS__); \ > + } \ > + if (_tst_ret != 0) { \ > + tst_brk_(FILENAME, LINENO, TBROK | TERRNO, \ > + CALLSTR " returned invalid value %d", \ > + ##__VA_ARGS__, _tst_ret); \ > + } \ > + _tst_ret; \ > + }) > + > +/* > + * assert that syscall returned any non-negative value (e.g. valid file > + * descriptor) > + */ > +#define TST_ASSERT_SYSCALL_FD(SCALL) \ > + TST_ASSERT_SYSCALL_FD_IMPL(SCALL, __FILE__, __LINE__, #SCALL) > + > +#define TST_ASSERT_SYSCALL_FD_IMPL(SCALL, FILENAME, LINENO, CALLSTR, ...) \ > + ({ \ > + int _tst_ret; \ > + errno = 0; \ > + _tst_ret = SCALL; \ > + if (_tst_ret == -1) { \ > + int _tst_ttype = errno == ENOTSUP ? TCONF : TBROK; \ > + tst_brk_(FILENAME, LINENO, _tst_ttype | TERRNO, \ > + CALLSTR " failed", ##__VA_ARGS__); \ > + } \ > + if (_tst_ret < 0) { \ > + tst_brk_(FILENAME, LINENO, TBROK | TERRNO, \ > + CALLSTR " returned invalid value %d", \ > + ##__VA_ARGS__, _tst_ret); \ > + } \ > + _tst_ret; \ > + }) I'd consider to add single implementation, which would be influenced with flags Something like if ((flags & TST_ASSERT_LT_0) && _tst_ret < 0 || _tst_ret != 0) \ But maybe some people will not like degreased readability with macro, and using flags would make it even a bit harder to read. Kind regards, Petr
Hi! > What I like on these macros (besides DRY) is that it really shows the test, not > the library, see > before: > tst_safe_timerfd.c:21: BROK: timerfd01.c:89 timerfd_create(CLOCK_MONOTONIC) failed: EINVAL (22) > after: > timerfd01.c:89: BROK: timerfd_create(CLOCK_MONOTONIC) failed: EINVAL (22) That's because it calls tst_brk_() correctly instead of tst_brk(). I should have caught that during the review. Also given the way it's structured now we can pass these parameters to a shell script as well and generate the end result easily. With a bit more work we can generate both header and C source as well and would still prefer that over these macros.
Hi, > > What I like on these macros (besides DRY) is that it really shows the test, not > > the library, see > > before: > > tst_safe_timerfd.c:21: BROK: timerfd01.c:89 timerfd_create(CLOCK_MONOTONIC) failed: EINVAL (22) > > after: > > timerfd01.c:89: BROK: timerfd_create(CLOCK_MONOTONIC) failed: EINVAL (22) > That's because it calls tst_brk_() correctly instead of tst_brk(). I > should have caught that during the review. OK. Again, sorry for that error. > Also given the way it's structured now we can pass these parameters to a > shell script as well and generate the end result easily. With a bit more > work we can generate both header and C source as well and would still > prefer that over these macros. OK, readability wins (I agree). Kind regards, Petr
diff --git a/include/tst_test.h b/include/tst_test.h index 8508c2e38..e423ceacc 100644 --- a/include/tst_test.h +++ b/include/tst_test.h @@ -281,6 +281,53 @@ extern void *TST_RET_PTR; TST_ERR = errno; \ } while (0) +/* assert that syscall returned only 0 and nothing else */ +#define TST_ASSERT_SYSCALL(SCALL) \ + TST_ASSERT_SYSCALL_IMPL(SCALL, __FILE__, __LINE__, #SCALL) + +#define TST_ASSERT_SYSCALL_IMPL(SCALL, FILENAME, LINENO, CALLSTR, ...) \ + ({ \ + int _tst_ret; \ + errno = 0; \ + _tst_ret = SCALL; \ + if (_tst_ret == -1) { \ + int _tst_ttype = errno == ENOTSUP ? TCONF : TBROK; \ + tst_brk_(FILENAME, LINENO, _tst_ttype | TERRNO, \ + CALLSTR " failed", ##__VA_ARGS__); \ + } \ + if (_tst_ret != 0) { \ + tst_brk_(FILENAME, LINENO, TBROK | TERRNO, \ + CALLSTR " returned invalid value %d", \ + ##__VA_ARGS__, _tst_ret); \ + } \ + _tst_ret; \ + }) + +/* + * assert that syscall returned any non-negative value (e.g. valid file + * descriptor) + */ +#define TST_ASSERT_SYSCALL_FD(SCALL) \ + TST_ASSERT_SYSCALL_FD_IMPL(SCALL, __FILE__, __LINE__, #SCALL) + +#define TST_ASSERT_SYSCALL_FD_IMPL(SCALL, FILENAME, LINENO, CALLSTR, ...) \ + ({ \ + int _tst_ret; \ + errno = 0; \ + _tst_ret = SCALL; \ + if (_tst_ret == -1) { \ + int _tst_ttype = errno == ENOTSUP ? TCONF : TBROK; \ + tst_brk_(FILENAME, LINENO, _tst_ttype | TERRNO, \ + CALLSTR " failed", ##__VA_ARGS__); \ + } \ + if (_tst_ret < 0) { \ + tst_brk_(FILENAME, LINENO, TBROK | TERRNO, \ + CALLSTR " returned invalid value %d", \ + ##__VA_ARGS__, _tst_ret); \ + } \ + _tst_ret; \ + }) + /* * Functions to convert ERRNO to its name and SIGNAL to its name. */
These macros take care of the standard return value checking boilerplate in cases where the test cannot continue after error. - TST_ASSERT_SYSCALL() calls tst_brk() if retval != 0 - TST_ASSERT_SYSCALL_FD() calls tst_brk() if retval < 0 Signed-off-by: Martin Doucha <mdoucha@suse.cz> --- Changes since v1: Added support for call args pretty printing in error message include/tst_test.h | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)