_endian.h: Include sys/ctypes.h for visibility macros
ClosedPublic
Actions

Authored by imp on Mar 20 2023, 9:12 PM.

Details

Reviewers

emaste
jrtc27
kib

Commits

rGd08ffbd70e72: _endian.h: Include sys/cdefs.h for visibility macros
rG4ccd5e64b76b: _endian.h: Include sys/cdefs.h for visibility macros
rGed52baf51bd1: _endian.h: Include sys/ctypes.h for visibility macros

Summary

BYTE_ORDER, LITTLE_ENDIAN and BIG_ENDIAN will be required by the
forthcoming POSIX Issue 8. In addition, they are provided in the BSD
compilation environments. However, depending on the order includes
happend, sys/cdefs.h may or may not be included when endian.h is
included. Include it here so we can safely test __BSD_VISIBLE. Add
visibility when we're compiling in the future for issue 8 (though it
is more of a tag of something to fix later).

This had the side effect of sometimes (in the traditional BSD
compliation environment)
#if BYTE_ORDER == LITTLE_ENDIAN
and
#if BYTE_ORDER == BIG_ENDIAN
both being true because none of these were defined. This fixes
that

Test Plan

this bug breaks at least librssl, which I've had reports that this patch fixes.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

imp requested review of this revision.Mar 20 2023, 9:12 PM

imp created this revision.

Harbormaster completed remote builds in B50488: Diff 119164.Mar 20 2023, 9:12 PM

imp added reviewers: emaste, jrtc27, kib.Mar 20 2023, 9:13 PM

imp edited the test plan for this revision. (Show Details)

Should it be cdefs.h in the review titie?

This revision is now accepted and ready to land.Mar 21 2023, 1:40 PM

I wondered if cdefs.h should be included from the user-facing includes that reference this one (i.e., sys/*/include/endian.h), but I'm fine with this.

sys/sys/_endian.h
75	so we're using anything past `200809` to mean Issue 8? I think it wouldn't hurt to extend this comment to clarify.

In D39176#892110, @kib wrote:

Should it be cdefs.h in the review titie?

Yes, that's right.

In D39176#892113, @emaste wrote:

I wondered if cdefs.h should be included from the user-facing includes that reference this one (i.e., sys/*/include/endian.h), but I'm fine with this.

We do it all over the place, to be honest. cdefs.h is designed to be invisible to user namespace, except when we're building for pre-ansi code, and it defines the ANSI keywords not in K&R in a nearly always sane way. That ship has sailed. It's the reason that I know this is a bug:

#include <endian.h>

fails, while

#include <stdio.h>
#include <endian.h>

works.

sys/sys/_endian.h
75	OK. The comment was supposed to do that, but I'll add a few more words to make it explicit.

Fix comment... will fix header to cdefs before commit since git arc isn't happy
when you change it and I'm not up for the manual workaround so many miles away
from home.

This revision now requires review to proceed.Mar 21 2023, 9:39 PM

Harbormaster completed remote builds in B50505: Diff 119216.Mar 21 2023, 9:39 PM

emaste accepted this revision.Mar 21 2023, 10:25 PM

This revision is now accepted and ready to land.Mar 21 2023, 10:25 PM

Closed by commit rGed52baf51bd1: _endian.h: Include sys/ctypes.h for visibility macros (authored by imp). · Explain WhyMar 22 2023, 2:26 AM

This revision was automatically updated to reflect the committed changes.

imp added a commit: rGed52baf51bd1: _endian.h: Include sys/ctypes.h for visibility macros.

imp added a commit: rG4ccd5e64b76b: _endian.h: Include sys/cdefs.h for visibility macros.Mar 22 2023, 10:31 PM

imp added a commit: rGd08ffbd70e72: _endian.h: Include sys/cdefs.h for visibility macros.Mar 23 2023, 6:41 AM