tests: Test endian.h, byteswap.h, sys/endian.h and both endian.h and byteswap.h together
Needs ReviewPublic
Actions

Authored by imp on Sep 21 2021, 5:08 PM.

Details

Reviewers

Summary

What's required and not required to be defined is complicated. Write
tests to enshrine it:
endian.h and sys/endian.h:

		[bl]e{16,32,64}toh
		hto[bl]e{16,32,64}

byteswap.h:

		{__,}bswap_{16,32,64}

sys/endian.h:

		{__,}bswap{16,32,64}
		_BYTE_ORDER
		_BIG_ENDIAN
		_LITTLE_ENDIAN
		_PDP_ENDIAN

endian.h:

		__BYTE_ORDER
		__BIG_ENDIAN
		__LITTLE_ENDIAN
		__PDP_ENDIAN
		__FLOAT_WORD_ORDER

NOT TESTED: deprecated symbols, internal to glibc symbols

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 41628
Build 38517: arc lint + arc unit

imp requested review of this revision.Sep 21 2021, 5:08 PM

imp created this revision.

If anybody knows an easier, less repetitive way to do this, I'm all ears.

Kyle made a suggestion to make it a lot simpler, use it.

include both orders for endian.h and sys/endian.h

arichardson added inline comments.

include/tests/sys_endian_test.c
22	Could use #error for these checks since it's done at compile time already?

include/tests/sys_endian_test.c
22	I don't think atf can give a proper count if you do that... I don't see other things in the tree do that...

include/tests/sys_endian_test.c
22	Yes, atf won't report it, but it will be reported (earlier) as a build failure.

This seems creating a new directory /usr/tests/include, do we also need to modify etc/mtree/BSD.tests.dist ?

In D32052#723399, @lwhsu wrote:

This seems creating a new directory /usr/tests/include, do we also need to modify etc/mtree/BSD.tests.dist ?

Yes. I hadn't through of that, but I can't imagine not needing it.

include/tests/sys_endian_test.c
22	I'll let others lead the way. There's just a couple of math tests that use it for a floating point size error...

rebase / tweak

include/Makefile
463	I suspect tests/include would be a more natural place for these tests. We generally try to pair tests with the corresponding executable or library sources, but I'm not sure that makes much sense here - it doesn't seem easy to find include/tests, and I'd naively expect that directory to just contain more header files, like other directories under include/.
include/tests/byteswap_test.c
24	style(9) says to avoid C++-style comments.
include/tests/sys_endian_endian_test.c
8 ↗	(On Diff #95588)
8 ↗	(On Diff #95588)	The comment should be formatted as a single block.

include/Makefile
463	Yea, I was a bit torn on this... One could make an argument too for some place under libc, but I didn't find a place to put them. I'll rename if it's easy, and leave them here if it isn't.
include/tests/byteswap_test.c
24	I think it used to say that, but doesn't anymore. It doesn't use them at all, and gives /* */ as the way to do one line comments I couldn't find the change that removed that advice, though. I'll fix this if I wind up moving things to tests/include.
include/tests/sys_endian_endian_test.c
8 ↗	(On Diff #95588)	OK.