Fix sys/opencrypto/blake2_test when kern.cryptodevallowsoft=0
Two of these testcases require software crypto to be enabled. Curiously, it
isn't by default.
PR: 230671
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
I like the general idea, but think we should add it to freebsd_test_suite macros (like ATF_REQUIRE_KERNEL_MODULE) since I am adding very similar tests right now and would like to use the same thing elsewhere.
It could also be made more generally useful, e.g., ATF_REQUIRE_SYSCTL_VALUE("kern.cryptodevallowsoft", 1). But that would require some sophistication to deal with differing sysctl sizes, so I am fine with the more restricted scope for now.
| tests/sys/opencrypto/blake2_test.c | ||
|---|---|---|
| 128 ↗ | (On Diff #46800) | (void) |
| 136 ↗ | (On Diff #46800) | I believe the "\n" here may actually break Kyua's parsing of ATF results. |
| 140 ↗ | (On Diff #46800) | style(9): Not a bool |
Comment Actions
create a common ATF_REQUIRE_SYSCTL_INT macro
The macro can't be general across all sysctl types because they can have
different sizes and types.
Comment Actions
LGTM with some questions below.
Sort of true, sort of not. sysctl(8) manages to be general across integer-type sysctls, and it's written in C. I'm ok with the less general approach for now, though.
| tests/freebsd_test_suite/macros.h | ||
|---|---|---|
| 34 ↗ | (On Diff #46804) | Where did this come from? |
| 56 ↗ | (On Diff #46804) | I don't see any reason this implementation needs to be a macro instead of an inline function. If there is no need, functions are preferred. |
| tests/freebsd_test_suite/macros.h | ||
|---|---|---|
| 34 ↗ | (On Diff #46804) | Uh, copy paste. I'm surprised it worked. It should've been sys/sysctl.h |
| 56 ↗ | (On Diff #46804) | I just copied the existing style of that file. And it looks to me like ngie didn't need to use macros either, but she did. Probably, she did it to match the existing style of the standard stuff in /usr/include/atf-c/macros.h. Those actually need to be macros, because they use __FILE__, and __LINE__. If you want to change it, let's change them all in a separate commit, and consult with ngie. |
| tests/freebsd_test_suite/macros.h | ||
|---|---|---|
| 56 ↗ | (On Diff #46804) | I don't think "uses macros" is a prevailing style for a file, nor that changing every unnecessary macro in a file is required to avoid introducing another, but whatever. I'd rather see it committed as a macro than not at all. (Ngie hasn't been active in FreeBSD for some time.) |
| tests/sys/opencrypto/blake2_test.c | ||
| 46 ↗ | (On Diff #46804) | This one can be dropped if the one in macros.h is fixed, right? |
| tests/sys/opencrypto/blake2_test.c | ||
|---|---|---|
| 46 ↗ | (On Diff #46804) | Yep, good catch. |