Page MenuHomeFreeBSD

Add a few atf tests for sendfile, with and without I/O errors.
ClosedPublic

Authored by chs on Jun 24 2020, 6:08 PM.

Details

Summary

Some atf tests will help us to avoid breaking sendfile's handling of I/O errors,
here are a few basic tests to get us started.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

chs requested review of this revision.Jun 24 2020, 6:08 PM

Thanks for doing this.

tests/sys/kern/sendfile_helper.c
43 ↗(On Diff #73590)

These are not sorted properly. sys/types.h should come first, followed by the rest of sys/*, followed by user headers sorted alphabetically.

@cem is this a useful test case for D25428?

48 ↗(On Diff #73590)

These can be declared static. The bools should probably be declared volatile as well, since they are accessed by multiple threads.

117 ↗(On Diff #73590)

The constant can be replaced with INADDR_LOOPBACK.

119 ↗(On Diff #73590)

It would be less fragile to allow the kernel to select an ephemeral port and then have the program get the bound port number using getsockname().

130 ↗(On Diff #73590)

These two lines can be replaced by errc(1, error, "pthread_create");

tests/sys/kern/sendfile_test.sh
27 ↗(On Diff #73590)

I would suggest adding a brief header comment here summarizing what these tests are covering.

tests/sys/kern/sendfile_helper.c
43 ↗(On Diff #73590)
$ clang-format -style=file testcase.c > testcase.c.new
$ diff -u testcase.c{,.new}
--- testcase.c  2020-06-24 11:45:15.287019000 -0700
+++ testcase.c.new      2020-06-24 11:45:32.647437000 -0700
@@ -1,14 +1,16 @@
 #include <sys/types.h>
-#include <err.h>
-#include <fcntl.h>
 #include <sys/socket.h>
-#include <netinet/in.h>
+#include <sys/sysctl.h>
 #include <sys/uio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
+
+#include <netinet/in.h>
+
+#include <err.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <pthread.h>
-#include <sys/sysctl.h>
 #include <stdbool.h>
 #include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>

update diff with markj's suggestions.

This revision is now accepted and ready to land.Jul 8 2020, 12:21 AM
This revision was automatically updated to reflect the committed changes.

Curious... is there a reason why this was added in addition to lib/libc/tests/sys/sendfile_test.c?

head/tests/sys/kern/sendfile_helper.c
103

What about IPv6?

In D25431#570827, @ngie wrote:

Curious... is there a reason why this was added in addition to lib/libc/tests/sys/sendfile_test.c?

I didn't learn of the existence of the sendfile test in libc until after I had written this new one.

head/tests/sys/kern/sendfile_helper.c
103

My reason for writing this sendfile test was to exercise the disk I/O error paths in sendfile, because I have had to fix bugs in those error paths 3 times in the last 6 months. Also, the sendfile code is complete agnostic to whether the socket is IPv4 or IPv6, so it did not occur to me to that there was any reason to test both versions of IP.