Add apisupport with two checks #445
Conversation
|
Depends on #446. |
|
In #83, you mused about adding support for other APIs (in particular, At the moment, this PR only uses it for detecting some nitpicky compiler things. In a few places, we don't strictly obey the C99 & POSIX standards, so some compilers legitimately complain about it. This PR does not add the capability for our code to do things like |
apisupport/Build/apisupport.sh
Outdated
| CFLAGS_HARDCODED="-D_POSIX_C_SOURCE=200809L -D_XOPEN_SOURCE=700" | ||
|
|
||
| feature() { | ||
| ARCH=$1 |
There was a problem hiding this comment.
Hmm, good point. I'm happy calling it PLATFORM, but at the moment, I'm using that variable to be NONPOSIX and LIBSSL. Is that stretching the meaning of "platform"?
There was a problem hiding this comment.
Maybe a bit of a stretch but it's better than anything else which comes to mind.
There was a problem hiding this comment.
ok, done as a REBASE commit.
|
Ok, LGTM. We should probably change the NO_SETGROUPS check to a APISUPPORT_NONPOSIX_SETGROUPS in setgroups_none.c at some point though? |
|
At some point, yes.
Although maybe "some point" should be "now". It would be easy to add that back in -- actually, the first 3 drafts of this PR contained that code, but I removed it since it wasn't needed. This gives me an excuse to get that infrastructure done now. |
On second thought, I'm not sure about this. Suppose that FreeBSD 14.0 moved
I'd say that "compile error" is the appropriate response. I'll still add the "support preprocessor |
|
Updated. I put the There's now a ton of |
apisupport/Build/apisupport.sh
Outdated
| FEATURE=$2 | ||
| EXTRALIB=$3 | ||
| shift 3; | ||
| if ! [ -f "${SRCDIR}"/apisupport-"$PLATFORM"-"$FEATURE".c ]; then |
There was a problem hiding this comment.
Excessive quotes (as in cpusupport.sh).
Specifically, I did: - s/cpu/api/g - s/CPU/API/g - s/ARCH/PLATFORM/g - removed the CPU features from the bottom of the file This commit does not include any run-time detection (which would occur in a new apisupport.h file).
It will be useful to allow checks to link to certain libraries (notably libssl).
This allows us to: - detect any compiler flags needed for special functionality - use those flags for specific files - use `#ifdef APISUPPORT_$platform_$feature` in the C preprocessor
setgroups() is not part of POSIX. We've hardcoded -D_POSIX_C_SOURCE=200809L
-D_XOPEN_SOURCE=700 into our command-line arguments, and we're trying to
undo that in util/setgroups_none.c by having:
/* We use non-POSIX functionality in this file. */
#undef _POSIX_C_SOURCE
#undef _XOPEN_SOURCE
However, that's not technically allowed under C99:
c99 7.1.3 Reserved identifiers:
All identifiers that begin with an underscore and either an uppercase
letter or another underscore are always reserved for any use.
...
If the program removes (with #undef) any macro definition of an
identifier in the first group listed above, the behavior is undefined.
The POSIX page for c99 states:
-D name[=value]
Define name as if by a C-language #define directive. If no = value
is given, a value of 1 shall be used. The -D option has lower
precedence than the -U option. That is, if name is used in both a -U
and a -D option, name shall be undefined regardless of the order of
the options.
...
-U name
Remove any initial definition of name.
In other words, `-U foo` is not the same thing as prepending `#undef foo`
to the top of the source file [1][2].
So our goal of this commit is to detect whether appending
-U_POSIX_C_SOURCE -U_XOPEN_SOURCE to the command-line will allow the
compiler to find setgroups().
[1] gcc and clang have slightly different descriptions of -D and -U: gcc
says that -U cancels any *previous* definition of foo, while clang says
that it adds an implicit #undef that is read before the source file is
processed. But those compiler's lack of POSIX compliance is not our
responsibility... and in any case, I'm fairly confident that is a
completely theoretical issue.
[2] FWIW, FreeBSD's `man c99` matches the POSIX behaviour for -U,
whereas `man cc` has the non-POSIX clang behaviour for -U.
openssl documents SSL_set_tlsext_host_name function prototype as:
int SSL_set_tlsext_host_name(const SSL *s, const char *name);
However, <openssl/tls1.h> implements it as:
# define SSL_set_tlsext_host_name(s,name) \
SSL_ctrl(s,SSL_CTRL_SET_TLSEXT_HOSTNAME,TLSEXT_NAMETYPE_host_name,\
(void *)name)
which drops the "const" from the "name" parameter. That's technically not
allowed, so the compiler might need to use a special flag for this file.
|
Fixed, rebased, ready for merge or more review. |
No description provided.