Oops, looks like this one's my fault. Thanks for fixing it, Mark!

In D17110#364972, @markj wrote:

In D17110#364950, @markj wrote:

In D17110#364949, @asomers wrote:

Do the tests still pass for you? After your latest change, I get the "connection wasn't accepted" error every time.

Yes, I just noticed that. (I rebuilt and ran the tests on two different machines by accident.) It seems like the polling loop is needed after all, which is somewhat surprising to me given that we're using a blocking connect().

I guess this is expected behaviour for TCP. connect(2) returns once the SYN|ACK reply is received, but the child socket isn't complete until the final ACK is received, so there's a window that we have to account for.

Do the tests still pass for you? After your latest change, I get the "connection wasn't accepted" error every time.

In D17115#364879, @vangyzen wrote:

In D17115#364864, @asomers wrote:

Two annoying questions:

1. The Wayback machine shows me that IANA sometimes removes a port assignment. Sadly, I haven't been able to find any official historical information. What will FreeBSD's policy regarding removing assignments be?

I doubt there has ever been such a policy. By default, we will remove it on the next update. I think this de facto policy will be sufficient. Administrators who need to keep the assignment can simply add it to this file on their systems.

All good here.

Two annoying questions:

In fact, you don't even need the second process for this test. It's possible to do this entirely with one single-threaded process. See the "accept_success" test in tests/sys/audit/network.c for an example. Basically it goes like this:

for (i=0; i<sdcnt; i++)
    listen(server_fd[i]);
while (true) {
    client_fd = socket();
    client_fd.connect(server_addr);  //blocking, but returns immediately
    for ((i=0; i<sdcnt; i++) {
        if (accept(server_fd[i]) == 0) {    // non-blocking
            acceptcnt[i]++;
            break;
        }
    }
}

What do you think of the single-threaded approach?

Thanks @cem. I know I can always count on you for a good review.

Fix an old typo in a comment

create a common ATF_REQUIRE_SYSCTL_INT macro

I use mergemaster the same way. Apart from the "default" word choice, this revision LGTM.

I don't see anything wrong with this review. However, since you're modifying the boot loader you should check with @imp.

What's anonymous tracing?

What's anonymous tracing?

Updating a Phabricator revision without including all of the files that have ever been part of that revision makes the history confusing, because Phabricator thinks you want to revert your changes to the files that you didn't include. You can't fix it now, but please try not to do it again.

LGTM. Don't forget to reference both PR 230175 and PR 230162 in your commit message. And don't forget to commit the autogenerated files too.

In D16525#350848, @kib wrote:

In D16525#350843, @asomers wrote:

The mprotect_err test is now failing on amd64. The assertion at line 167 fails. I haven't tried it in 32-bit mode yet; I need to rebuild my 32-bit world first.

I see what is going on, finally. Can you explain to me how to rebuild mprotect_test without rebuilding the whole world, even with -DNO_CLEAN ? For normal components, I can make buildenv, then cd <dir> && make.

The mprotect_err test is now failing on amd64. The assertion at line 167 fails. I haven't tried it in 32-bit mode yet; I need to rebuild my 32-bit world first.

Also, I notice three other syscalls in compat/freebsd32 that use plain timespec instead of timespec32. Do they need fixing too? I haven't tested them. They are freebsd32_sigtimedwait, freebsd32_futimens, and freebsd32_utimensat.

Are you going to regenerate sys/compat/freebsd32 stuff too?

Abandoning revision because I didn't get approval before I lost access to the test environment.

Committed by r336914. I don't know why Phabricator didn't close it automatically.

Abandoning the revision because I no longer have relevant hardware to develop on. @slm can commandeer the revision if he wants to.

The poll passed by 82%, so I'm going to commit it. I've updated the diff at https://reviews.freebsd.org/D13465 and I'll commit in a few days if nobody finds any problems. As well as man, this version of the diff updates other utilities, too.

Additionally change the defaults for mandoc, apropos, ftp, mail, msgs,
mergemaster, and freebsd-update. And change the default PAGER variable in
/usr/share/skel, /root, and tools/tools/nanobsd.

In D16500#350454, @jhb wrote:

I'm confused by this change. Why did it not work before? Did the != NULL check not work so that it was never calling copyout? Otherwise, it was calling copyout correctly I don't see how EFAULT would not have been returned.

Respond to comments