Page MenuHomeFreeBSD

uexterr_gettext: add tests
AbandonedPublic

Authored by asomers on Sat, Jul 5, 9:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Jul 8, 5:57 AM
Unknown Object (File)
Tue, Jul 8, 12:29 AM
Unknown Object (File)
Tue, Jul 8, 12:07 AM
Unknown Object (File)
Mon, Jul 7, 9:00 AM

Details

Reviewers
arrowd
kib
andrew
shurd
manu
brooks
olce
Group Reviewers
cam
Summary

Add tests for the new extended errno feature, using a custom kernel
module to report errors to userland.

Sponsored by: ConnectWise
MFC after: 2 weeks

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Warnings
SeverityLocationCodeMessage
Warningtests/sys/net/if_bridge_test.sh:CHMOD1Invalid Executable
Unit
No Test Coverage
Build Status
Buildable 65240
Build 62123: arc lint + arc unit

Event Timeline

tests/sys/exterr/exterr_test.c
121 ↗(On Diff #158032)

This scenario is what motivated me to write the tests. I'm glad to see that it works already.

Loading module which is not configured against the same kernel config as current kernel is dangerous. It does not guarantee that the KBI of module and kernel match, and then all bets are off.

I suggest you to utilize existing syscalls instead: we already have almost whole top-level mmap(2) converted to exterr. Then, I guarantee that exterrctl(2) would never set the ext error itself. So you could
use exterrctl(2) with non-existing command code to cause error without exterr. I would even suggest to add e.g. EXTERRCTL_UD command that is guaranteed to return EINVAL.

tests/sys/exterr/exterr_test.c
121 ↗(On Diff #158032)

Right. exterr_copyout() is called if the current thread registered the uexterr region, and there is an error result from syscall. Then exterr_copyout() explicitly set uexterr.error to zero if there is no extended error data.

tests/sys/exterr/exterr_test_kld.c
26 ↗(On Diff #158032)

types.h is not needed since param.h is included

36 ↗(On Diff #158032)

Except for param.h that must go first, please order includes alphabetically

76 ↗(On Diff #158032)

If make_dev_p resulted in error, this line would deref NULL.

105 ↗(On Diff #158032)
  • Abandon the custom kernel module. Rely on existing syscalls instead.
  • move the test into the kern directory

I have no idea why Arcanist decided to include all of those extra revisions when I updated the diff. I did not rebase. Only the most recent three revisions are intended to be part of the review. I'll abandon and reopen.