Fix problems in the kern_maxfiles__increase test

[vangyzen]

Summary

ATF functions such as ATF_REQUIRE do not work correctly in child processes.
Use plain C functions to report errors instead.

In the parent, check for the untimely demise of children. Without this,
the test hung until the framework's timeout.

Raise the resource limit on the number of open files. If this was too low,
the test hit the two problems above.

Restore the kern.maxfiles sysctl OID in the cleanup function.
The body prematurely removed the symlink in which the old value was saved.

Sort includes.

Clean up a temporary file created by the test ("afile").

Test Plan

$ ulimit -n
234792
$ kyua test kern_descrip_test:kern_maxfiles__increase
kern_descrip_test:kern_maxfiles__increase  ->  passed  [0.369s]

Results file id is usr_tests_sys_kern.20191004-214922-978634
Results saved to /root/.kyua/store/results.usr_tests_sys_kern.20191004-214922-978634.db

1/1 passed (0 failed)
$ ulimit -n 1000
$ ulimit -n
1000
$ kyua test kern_descrip_test:kern_maxfiles__increase
kern_descrip_test:kern_maxfiles__increase  ->  passed  [0.420s]

Results file id is usr_tests_sys_kern.20191004-214932-330605
Results saved to /root/.kyua/store/results.usr_tests_sys_kern.20191004-214932-330605.db

1/1 passed (0 failed)

[asomers]

That's not much of a test plan. Perhaps try sudo?

tests/sys/kern/kern_descrip_test.c

107 ↗

(On Diff #62921)

FWIW Coverity is going to complain about a file descriptor leak here. I've been working with @uqs and @koobs to find a way to annotate these leaks, but so far nothing will make Coverity shut up.

133 ↗

(On Diff #62921)

If you remove WNOHANG, then, the 1000us sleep would not be necessary.

138 ↗

(On Diff #62921)

What's the point of this sleep?

182 ↗

(On Diff #62921)

I think the test would be more robust if you changed 1 to EXPAND / 2.

199 ↗

(On Diff #62921)

This should be unnecessary. The temporary files are contained within Kyua's temporary directory, which will be automatically cleaned up.

[vangyzen]

That's not much of a test plan. Perhaps try sudo?

No kidding. I ran it in the wrong terminal window and didn't notice until right _after_ I created the review. (The testing during coding was run as root.)

tests/sys/kern/kern_descrip_test.c

107 ↗

(On Diff #62921)

Thank you. Coverity is really helpful, but training it to avoid FPs seems to take a lot of work.

133 ↗

(On Diff #62921)

The test would hang if I did that. The children don't exit until the parent unlinks the RENDEZVOUS file. The children need to hit their max file descriptor count simultaneously in order for the test to be valid.

138 ↗

(On Diff #62921)

Presumably to let all the children exit before the parent does. It wasn't broken before, so I didn't fix it.

182 ↗

(On Diff #62921)

I'll consider it.

199 ↗

(On Diff #62921)

It's unnecessary, but I consider it good form.

[vangyzen]

• fixup: make test more robust by opening more files

[vangyzen]

• rebase

[vangyzen]

tests/sys/kern/kern_descrip_test.c

182 ↗

(On Diff #62921)

As it turns out, your suggestion is essential for the validity of the test, due to the integer division on line 131. Thanks!

[asomers]

tests/sys/kern/kern_descrip_test.c

133 ↗

(On Diff #62921)

Oh, I get it. RENDEZVOUS is a hokey barrier. You could do this more elegantly with pthread_barrier_wait and friends.

138 ↗

(On Diff #62921)

But it doesn't actually do that. It just creates a race. I think you should either delete it, or fix it by using proper synchronization primitives.

[vangyzen]

• wait() for children instead of a sleep race

[vangyzen]

tests/sys/kern/kern_descrip_test.c

133 ↗

(On Diff #62921)

I could, but I should move on to other work. Feel free, if you're interested.

138 ↗

(On Diff #62921)

You mean it doesn't guarantee that. In practice, it probably did let the children exit. Even if it didn't, the race was harmless.

I didn't want to put any more effort into this test, but since you found a nice bug, I'll return the favor. :)