Page MenuHomeFreeBSD

ptrace_test: Add more debug output on test failures
ClosedPublic

Authored by arichardson on Feb 23 2021, 12:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 14, 1:17 AM
Unknown Object (File)
Thu, Mar 14, 1:17 AM
Unknown Object (File)
Thu, Mar 14, 1:17 AM
Unknown Object (File)
Thu, Mar 14, 1:17 AM
Unknown Object (File)
Thu, Mar 14, 1:17 AM
Unknown Object (File)
Thu, Mar 14, 1:04 AM
Unknown Object (File)
Jan 14 2024, 6:07 PM
Unknown Object (File)
Jan 6 2024, 9:12 PM
Subscribers

Details

Summary

Mostly automatic, using
CHILD_REQUIRE\(([^|&\n]*) == -> CHILD_REQUIRE_EQ_INT($1,
ATF_REQUIRE\(([^|&\n]*) == -> REQUIRE_EQ_INT($1, followed by
git-clang-format -f and then manually checking ones that contain ||/&&.

PR: 243605

Test Plan

still getting the same failure but now it prints psr.sr_error (0) == EBADF (9) not met instead of just failing without printing the values.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tests/sys/kern/ptrace_test.c
92–105

Per style(9) every line after the #define should be deindented by one tab.

112

s/2/STDERR_FILENO/? Why not use fprintf(stderr)?

129

Ditto.

tests/sys/kern/ptrace_test.c
92–105

I tried to match the macro above, will remove one leading tab in both.

112

I'm not sure why the previous code didn't use fprintf(). I just replaced the write(2, buf, strlen(buf)) with dprintf, but being able to use vfprintf() would be even simpler. I'll defer to @jhb who appears to have written this.

arichardson marked 2 inline comments as done.
  • fix macro indentation

Could you just use ATF_REQUIRE_EQ directly instead of the local REQUIRE_EQ_INT? Then dropping _INT from CHILD_REQUIRE_EQ() would be more symmetric to the ATF API.

tests/sys/kern/ptrace_test.c
98

I would be tempted to drop the _INT suffix since you are using __typeof__, etc. I would otherwise read _INT as meaning the arguments are of type int.

104

Maybe s/long long/intmax_t/? We tend to use [u]intmax_t and %j for printing arbitrary integers rather than long long.

112

The reason it used write(2) previously was to avoid dealing with any unflushed stdio buffers after fork() I think. I feel like I got duplicate output previously as I probably wouldn't have resorted to that just due to paranoia.

arichardson marked 4 inline comments as done.

Apply review suggestions

tests/sys/kern/ptrace_test.c
98

Yeah I was thinking it means "any type that can meaningfully be cast to an integer". Will update the diff to remove the suffix shortly.

Unless there are any further suggestions, I'll commit this tomorrow (since there are two dependent changes that unbreak the testsuite).

tests/sys/kern/ptrace_test.c
124

I think you still don't need this and can just use ATF_REQUIRE_EQ() directly instead?

tests/sys/kern/ptrace_test.c
124

The reason I did this is that ATF_REQUIRE_EQ is stupid and doesn't print any useful information.

jhb added inline comments.
tests/sys/kern/ptrace_test.c
124

Humm, maybe we should fix those to include the values as well as the source name? I guess it is easy to convert these in the future if someone fixes that. A proper ATF_REQUIRE_EQ would have to do some switches on types though to use %p for pointers vs %jd for scalars and something else for floats.

This revision is now accepted and ready to land.Mar 1 2021, 6:41 PM