Add sigsuspend #2279

Toru3 · 2024-01-03T05:30:24Z

What does this PR do

Add sigsuspend as SigSet::suspend().

Checklist:

I have read CONTRIBUTING.md
I have written necessary tests and rustdoc comments
A change log has been added if this PR modifies nix's API

Related #1994 #1997

> Since sigsuspend() suspends thread execution indefinitely, there is no > successful completion return value. If a return occurs, -1 shall be > returned and errno set to indicate the error. https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigsuspend.html

This reverts commit c174bd2.

This reverts commit f63e656.

SteveLauC

Thanks for your contribution, generally looks good! I have left some comments

SteveLauC · 2024-01-06T07:28:36Z

src/sys/signal.rs

+        match Errno::result(res).map(drop) {
+            Err(Errno::EINTR) => Ok(()),
+            Err(e) => Err(e),
+            Ok(_) => unreachable!(),


It might be better to slightly state why the Ok() branch is unreachable:

Suggested change

Ok(_) => unreachable!(),

Ok(_) => unreachable!("because this syscall always returns -1 if returns"),

I added reason.

SteveLauC · 2024-01-06T07:33:55Z

src/sys/signal.rs

+        target_os = "linux",
+        target_os = "android",


We prefer to use alias if available:

Suggested change

target_os = "linux",

target_os = "android",

linux_android,

I fixed it.

SteveLauC · 2024-01-06T07:34:32Z

src/sys/signal.rs

+        target_os = "linux",
+        target_os = "android",
+        target_os = "hurd",
+        target_os = "nto",


Please remove this as we are not sure if we should support this target

I removed nto.

SteveLauC · 2024-01-06T07:35:10Z

test/sys/test_signal.rs

+    target_os = "linux",
+    target_os = "android",


Same here, use alias please.

I fixed it.

SteveLauC · 2024-01-06T07:35:20Z

test/sys/test_signal.rs

+    target_os = "linux",
+    target_os = "android",
+    target_os = "hurd",
+    target_os = "nto",


Please remove this

I removed nto.

SteveLauC · 2024-01-06T08:15:26Z

Never mind about the CI / tier3 (x86_64-unknown-dragonfly) (pull_request) failure, it is not related to this PR

asomers

Please wait for rust-lang/libc#3518 to merge. Then update Nix's Cargo.toml to point to the merged revision and enable sigsuspend for the additional platforms.

SteveLauC · 2024-01-07T01:58:24Z

Please wait for rust-lang/libc#3518 to merge. Then update Nix's Cargo.toml to point to the merged revision and enable sigsuspend for the additional platforms.

That PR is merged, @Toru3, you can update our libc dependency and add sigsuspend for more targets:

[dependencies]
-libc = { git = "https://github.com/rust-lang/libc", rev = "cb18b837963c37a8d21732f3ca2c2096f04e6830", features = ["extra_traits"] }
+libc = { git = "https://github.com/rust-lang/libc", rev = "2f93bfb7678e18a9fc5373dec49384bd23f601c3", features = ["extra_traits"] }

#[cfg(any(
        bsd,
        linux_android,
        solarish,
        target_os = "haiku",
        target_os = "hurd",
        target_os = "aix",
        target_os = "fushsia"
    ))]

I updated libc to use below commit which add sigsuspend to more targets. rust-lang/libc@11f7c7b

SteveLauC · 2024-01-07T03:23:40Z

~~You may want to rebase your branch to solve the DragonFlyBSD CI error~~

Never mind, the CI seems to be good

Toru3 · 2024-01-07T03:37:07Z

@asomers I updated libc and added targets to sigsuspend. Please review this.

SteveLauC

It is all good, let's merge this:)

Toru3 added 18 commits January 3, 2024 11:51

add sigsuspend

79e5e94

add cfg for sigsuspend

955d4ef

add fuchsia to target_os

b2936c7

Ok is unreachable

bc724a7

> Since sigsuspend() suspends thread execution indefinitely, there is no > successful completion return value. If a return occurs, -1 shall be > returned and errno set to indicate the error. https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigsuspend.html

add test_sigsuspend

aaaec15

clippy

bc7b717

fix bug of test

aed44de

add changelog

2d9631d

update doc

599aa53

remove unnnessesary sleep

c174bd2

Revert "remove unnnessesary sleep"

d5374fa

This reverts commit c174bd2.

move to SigSet::suspend

80f3d01

reorder doc

10c1db7

update test

9e17085

update test

9f52f57

change memory order

f63e656

Revert "change memory order"

67f8673

This reverts commit f63e656.

rename added.md

4e3b666

SteveLauC requested changes Jan 6, 2024

View reviewed changes

Toru3 added 3 commits January 6, 2024 17:07

add unreachable reason

55c49fe

use alias

682e4e9

remove nto

5a7a3fe

SteveLauC approved these changes Jan 6, 2024

View reviewed changes

asomers requested changes Jan 6, 2024

View reviewed changes

SteveLauC linked an issue Jan 7, 2024 that may be closed by this pull request

hints for adding sigsuspend(). #1994

Closed

SteveLauC mentioned this pull request Jan 7, 2024

Add suspend to sys::signal::SigSet #1997

Closed

Toru3 added 2 commits January 7, 2024 12:12

Update libc

a02d066

I updated libc to use below commit which add sigsuspend to more targets. rust-lang/libc@11f7c7b

Add target bsd, solarish and haiku to sigsuspend

a20176c

SteveLauC approved these changes Jan 7, 2024

View reviewed changes

SteveLauC added this pull request to the merge queue Jan 7, 2024

Merged via the queue into nix-rust:master with commit 4bc85e4 Jan 7, 2024
35 checks passed

SteveLauC added a commit to SteveLauC/nix that referenced this pull request Jan 7, 2024

chore: move the changelog of nix-rust#2279 to right place

db8d7ff

SteveLauC mentioned this pull request Jan 7, 2024

chore: move the changelog of #2279 to right place[skip ci] #2285

Merged

3 tasks

SteveLauC added a commit that referenced this pull request Jan 7, 2024

chore: move the changelog of #2279 to right place (#2285)

24b70dc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add sigsuspend #2279

Add sigsuspend #2279

Toru3 commented Jan 3, 2024 •

edited

SteveLauC left a comment

SteveLauC Jan 6, 2024

Toru3 Jan 6, 2024

SteveLauC Jan 6, 2024

Toru3 Jan 6, 2024 •

edited

SteveLauC Jan 6, 2024

Toru3 Jan 6, 2024

SteveLauC Jan 6, 2024

Toru3 Jan 6, 2024

SteveLauC Jan 6, 2024

Toru3 Jan 6, 2024

SteveLauC commented Jan 6, 2024

asomers left a comment

SteveLauC commented Jan 7, 2024

SteveLauC commented Jan 7, 2024 •

edited

Toru3 commented Jan 7, 2024

SteveLauC left a comment

	Ok(_) => unreachable!(),
	Ok(_) => unreachable!("because this syscall always returns -1 if returns"),

Add sigsuspend #2279

Add sigsuspend #2279

Conversation

Toru3 commented Jan 3, 2024 • edited

What does this PR do

Checklist:

SteveLauC left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Toru3 Jan 6, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteveLauC commented Jan 6, 2024

asomers left a comment

Choose a reason for hiding this comment

SteveLauC commented Jan 7, 2024

SteveLauC commented Jan 7, 2024 • edited

Toru3 commented Jan 7, 2024

SteveLauC left a comment

Choose a reason for hiding this comment

Toru3 commented Jan 3, 2024 •

edited

Toru3 Jan 6, 2024 •

edited

SteveLauC commented Jan 7, 2024 •

edited