Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hurd build fixes from Debian #627

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+3 −5
Open

Hurd build fixes from Debian #627

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
179338c
Fix build on GNU Hurd: define _GNU_SOURCE so unistd.h declares pipe2.
markhindley Apr 23, 2023
9f6cb01
Fix build on GNU Hurd: kvm library is BSD only.
markhindley Jul 25, 2022
b2c6b66
Update meson.build
markhindley Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ else
os = option_os
endif

if os != 'Linux'
if os == 'Dragonfly' or os == 'FreeBSD' or os == 'NetBSD' or os == 'OpenBSD'
markhindley marked this conversation as resolved.
Show resolved Hide resolved
kvm_dep = cc.find_library('kvm', required: true)
else
kvm_dep = []
Expand Down
6 changes: 2 additions & 4 deletions src/start-stop-daemon/start-stop-daemon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,8 @@

#define ONE_MS 1000000

#ifdef __linux__
/* For extra SCHED_* defines. */
# define _GNU_SOURCE
#endif
/* For extra SCHED_* defines and pipe2. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

#define _GNU_SOURCE

#include <errno.h>
#include <fcntl.h>
Expand Down