Hurd build fixes from Debian #627
Conversation
meson.build
Outdated
| endif | ||
|
|
||
| if os != 'Linux' | ||
| if os != 'Linux' and os != 'GNU' |
There was a problem hiding this comment.
not a new issue, but this check seems to assume that we always want kvm on non-Linux systems. this should really be changed to only require kvm on BSD systems
| #ifdef __linux__ | ||
| /* For extra SCHED_* defines. */ | ||
| /* For extra SCHED_* defines and pipe2. */ | ||
| # define _GNU_SOURCE |
|
|
||
| #ifdef __linux__ | ||
| /* For extra SCHED_* defines. */ | ||
| /* For extra SCHED_* defines and pipe2. */ |
There was a problem hiding this comment.
the pipe2 usage implies a bigger problem here i think. we're using it unprotected. we really need a test in meson to see if the system supports pipe2 and switch between it & standard pipe.
There was a problem hiding this comment.
while this is correct, OTOH it seems that pipe2() is available in all the OSes for which there is some kind of support in OpenRC:
- Linux & Hurd: provided by glibc
- FreeBSD: https://man.freebsd.org/cgi/man.cgi?query=pipe2&sektion=2&n=1
- OpenBSD: https://man.openbsd.org/pipe.2
- NetBSD: https://man.netbsd.org/pipe2.2
- DragonFlyBSD: https://man.bsd.lv/DragonFly-5.6.1/man2/pipe2.2
so adding a meson test for it with fallback to pipe() would effectively create dead code, because it would not be used on any of the supported OSes; considering that currently pipe2() is used unconditionally without issues, IMHO it would be acceptable to keep using it like that, and wait until anyone tries to port OpenRC to another OS
There was a problem hiding this comment.
see if the system supports pipe2 and switch between it & standard pipe.
I haven't taken a look into the usage yet, so this might not be a problem, but pipe2 and pipe + fcntl aren't functionally equivalent.
With pipe + fcntl there exists a window between these two calls where the fd might not have proper mode set. This can become an issue in multi-threaded or signal handler contexts.
markhindley
changed the title
FAILED: src/start-stop-daemon/start-stop-daemon.p/start-stop-daemon.c.o
cc -Isrc/start-stop-daemon/start-stop-daemon.p -Isrc/start-stop-daemon -I../src/start-stop-daemon -Isrc/shared -I../src/shared -Isrc/libeinfo -I../src/libeinfo -Isrc/librc -I../src/librc -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Wpedantic -std=c99 -D_DEFAULT_SOURCE -DMAXPATHLEN=4096 -DPATH_MAX=4096 -Wcast-align -Wcast-qual -Wdeclaration-after-statement -Wformat=2 -Winline -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wsequence-point -Wshadow -Wwrite-strings -Werror=implicit-function-declaration -DHAVE_MALLOC_EXTENDED_ATTRIBUTE -DHAVE_CLOSEFROM -DHAVE_CLOSE_RANGE_CLOEXEC -g -O2 -ffile-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -MD -MQ src/start-stop-daemon/start-stop-daemon.p/start-stop-daemon.c.o -MF src/start-stop-daemon/start-stop-daemon.p/start-stop-daemon.c.o.d -o src/start-stop-daemon/start-stop-daemon.p/start-stop-daemon.c.o -c ../src/start-stop-daemon/start-stop-daemon.c
../src/start-stop-daemon/start-stop-daemon.c: In function ‘main’:
../src/start-stop-daemon/start-stop-daemon.c:865:13: error: implicit declaration of function ‘pipe2’; did you mean ‘pipe’? [-Werror=implicit-function-declaration]
865 | if (pipe2(pipefd, O_CLOEXEC) == -1)
| ^~~~~
| pipe
Although this is triggered on HURD, according to glibc pipe2(2), this is
required for the 2 argument form irresepctive of arch.
SYNOPSIS
#include <unistd.h>
int pipe(int pipefd[2]);
#define _GNU_SOURCE /* See feature_test_macros(7) */
#include <fcntl.h> /* Definition of O_* constants */
#include <unistd.h>
int pipe2(int pipefd[2], int flags);
../meson.build:47:2: ERROR: C shared or static library 'kvm' not found
|
these commits are better and i don't think make the status quo worse, but i still think we should fix the regression around pipe2 rather than make it a hard requirement |
Co-authored-by: Pino Toscano <toscano.pino@tiscali.it>
Is it a regression though since it's supported by all OSes we care about? It doesn't seem worthwhile to me to add a compatibility wrapper which will almost certainly never be used (and thus never tested). It's likely more fruitful to propose to austingroup to standardize pipe2 in posix instead (which I might do this weekend). IMO, we should merge this. It fixes actual build failure and doesn't worsen anything. |
|
this PR unconditionally defines |
As far as I'm aware it just makes certain prototype/defines etc visible on glibc and other libcs which honor that define. Does it actually have any downside when defined in BSDs? I see many projects unconditionally defining it (and other similar feature test macros) without any issues. |
There was already one unconditional (and unnecessary) |
|
@markhindley considering that PR #801 (thanks @N-R-K!) solved the Would you please update this PR by rebasing it, dropping all the changes except the ones in |
#857 already includes the missing fixes (by virtue of re-working a lot of the meson.build files). please test that PR instead. |
PR as requested in #626
Mark