Hurd build fixes from Debian #627
Conversation
meson.build
Outdated
| @@ -43,7 +43,7 @@ else | |||
| os = option_os | |||
| 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
| @@ -21,10 +21,8 @@ | |||
|
|
|||
| #define ONE_MS 1000000 | |||
|
|
|||
| #ifdef __linux__ | |||
| /* For extra SCHED_* defines. */ | |||
| /* For extra SCHED_* defines and pipe2. */ | |||
| # define _GNU_SOURCE | |||
| @@ -21,10 +21,8 @@ | |||
|
|
|||
| #define ONE_MS 1000000 | |||
|
|
|||
| #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 <[email protected]>
PR as requested in #626
Mark