Skip to content

Add 0 to SIG enum #26012

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

Closed
psznm wants to merge 5 commits into from
+43 −0
Closed

Add 0 to SIG enum #26012

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9781e02
Add 0 to SIG enum
psznm Nov 22, 2025
5aff6cd
Add test for kill 0
psznm Nov 23, 2025
2f738eb
Standalone kill test
psznm Nov 24, 2025
cdb85b2
Limit test to OS with known maximum PID. Omit windows from kill 0 test
psznm Nov 24, 2025
a79ccf9
use testing.expectError
psznm Nov 24, 2025
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
8 changes: 8 additions & 0 deletions lib/std/c.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2662,6 +2662,8 @@ pub const SIG = switch (native_os) {
pub const IOT: SIG = .ABRT;
pub const POLL: SIG = .EMT;

/// Invalid signal. Used in kill to perform checking without sending signal.
INVAL = 0,
/// hangup
HUP = 1,
/// interrupt
Expand Down Expand Up @@ -2756,6 +2758,7 @@ pub const SIG = switch (native_os) {
pub const RTMIN = 65;
pub const RTMAX = 126;

INVAL = 0,
HUP = 1,
INT = 2,
QUIT = 3,
Expand Down Expand Up @@ -2821,6 +2824,7 @@ pub const SIG = switch (native_os) {

pub const POLL: SIG = .IO;

INVAL = 0,
HUP = 1,
INT = 2,
QUIT = 3,
Expand Down Expand Up @@ -2895,6 +2899,7 @@ pub const SIG = switch (native_os) {

pub const IOT: SIG = .ABRT;

INVAL = 0,
HUP = 1,
INT = 2,
QUIT = 3,
Expand Down Expand Up @@ -2941,6 +2946,7 @@ pub const SIG = switch (native_os) {

pub const IOT: SIG = .ABRT;

INVAL = 0,
HUP = 1,
INT = 2,
QUIT = 3,
Expand Down Expand Up @@ -2989,6 +2995,7 @@ pub const SIG = switch (native_os) {

pub const IOT: SIG = .ABRT;

INVAL = 0,
HUP = 1,
INT = 2,
QUIT = 3,
Expand Down Expand Up @@ -3035,6 +3042,7 @@ pub const SIG = switch (native_os) {

pub const IOT: SIG = .ABRT;

INVAL = 0,
HUP = 1,
INT = 2,
QUIT = 3,
Expand Down
6 changes: 6 additions & 0 deletions lib/std/os/linux.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3741,6 +3741,8 @@ pub const SIG = if (is_mips) enum(u32) {
pub const IOT: SIG = .ABRT;
pub const POLL: SIG = .IO;

INVAL = 0,

// /arch/mips/include/uapi/asm/signal.h#L25
HUP = 1,
INT = 2,
Expand Down Expand Up @@ -3787,6 +3789,8 @@ pub const SIG = if (is_mips) enum(u32) {
pub const PWR: SIG = .LOST;
pub const POLL: SIG = .IO;

/// Perform error checking without sending signal.
INVAL = 0,
HUP = 1,
INT = 2,
QUIT = 3,
Expand Down Expand Up @@ -3830,6 +3834,8 @@ pub const SIG = if (is_mips) enum(u32) {
pub const POLL: SIG = .IO;
pub const IOT: SIG = .ABRT;

/// Perform error checking without sending signal.
INVAL = 0,
HUP = 1,
INT = 2,
QUIT = 3,
Expand Down
2 changes: 2 additions & 0 deletions lib/std/os/plan9.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ pub fn getpid() u32 {
return tos.pid;
}
pub const SIG = struct {
/// Invalid signal. Used in kill to perform checking without sending signal.
pub const INVAL = 0;
/// hangup
pub const HUP = 1;
/// interrupt
Expand Down
3 changes: 3 additions & 0 deletions test/standalone/posix/build.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ const cases = [_]Case{
.{
.src_path = "relpaths.zig",
},
.{
.src_path = "kill.zig",
},
};

pub fn build(b: *std.Build) void {
Expand Down
20 changes: 20 additions & 0 deletions test/standalone/posix/kill.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
const std = @import("std");
const posix = std.posix;

pub fn main() !void {
try test_kill_zero_self_should_succeed();
try test_kill_nonexistent();
}

fn test_kill_nonexistent() !void {
const impossible_pid: posix.pid_t = 1_999_999_999;
posix.kill(impossible_pid, .INVAL) catch |err| switch (err) {
Copy link
Contributor

@rootbeer rootbeer Nov 24, 2025

Choose a reason for hiding this comment

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

I think you could use std.testing.expectError() here to simplify a bit:

std.testing.expectError(posix.KillError.ProcessNotFound, posix.kill(impossible_pid, .INVAL));

Copy link
Author

@psznm psznm Nov 24, 2025

Choose a reason for hiding this comment

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

Thanks! I'll have to inspect std.testing more.

posix.KillError.ProcessNotFound => return,
else => return err,
};
return error.ProcessShouldHaveNotBeenFound;
}
Comment on lines 11 to 18
Copy link
Member

@alexrp alexrp Nov 24, 2025

Choose a reason for hiding this comment

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

All it takes is a write to /proc/sys/kernel/pid_max on Linux to make this test unreliable.

Unless you can come up with a non-flaky and portable way of obtaining a truly "impossible" PID, this part of the test can't be merged.

Copy link
Author

@psznm psznm Nov 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

All it takes is a write to /proc/sys/kernel/pid_max on Linux to make this test unreliable.

Unless you can come up with a non-flaky and portable way of obtaining a truly "impossible" PID, this part of the test can't be merged.

I think you might be wrong. There appears to be a constant in linux PID_MAX_LIMIT which limits that number to somewhere around 4 million. https://github.com/torvalds/linux/blob/master/include/linux/threads.h#L34

Attempting # echo "4194305" > /proc/sys/kernel/pid_max confirms that, at least for linux. I am not knowledgable of other posix systems, but even then it seems better to have this test that is extremely unlikely to ever be flakey, than to not have the test at all?

Copy link
Member

@alexrp alexrp Nov 24, 2025

Choose a reason for hiding this comment

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

We can have the test if it's only enabled for operating systems on which it can be verified that the PID is in fact impossible. We have a pretty strict "no flaky tests" policy.

Copy link
Author

@psznm psznm Nov 24, 2025

Choose a reason for hiding this comment

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

I have limited the test to linux (limited by the constant) and macos (results of searching generally agree on a non-configurable and verifiable number)

I have also excluded windows from the test because it apparently does not have kill -0 equivalent

Copy link
Contributor

@rootbeer rootbeer Nov 24, 2025

Choose a reason for hiding this comment

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

Given this is testing Zig's handling of the error path (vs. testing the OS's error path), it seems sufficient (and safe) to make this a (native_os == .linux) only test. Then you can be confident in the PID_MAX_LIMIT, and we exercise the Zig code well enough?

Copy link
Contributor

@rootbeer rootbeer Nov 24, 2025

Choose a reason for hiding this comment

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

Apologies, github didn't show me your most recent update, which basically does what I suggested, plus MacOS.

Copy link
Author

@psznm psznm Nov 24, 2025

Choose a reason for hiding this comment

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

Given this is testing Zig's handling of the error path (vs. testing the OS's error path), it seems sufficient (and safe) to make this a (native_os == .linux) only test. Then you can be confident in the PID_MAX_LIMIT, and we exercise the Zig code well enough?

Thanks for the suggestion, I have already limited to linux and macos. both of which I am confident of never being able to hit the PID limit, if that is fine?

Copy link
Author

@psznm psznm Nov 24, 2025

Choose a reason for hiding this comment

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

Apologies, github didn't show me your most recent update, which basically does what I suggested, plus MacOS.

Thanks for the suggestion, I have already limited to linux and macos. both of which I am confident of never being able to hit the PID limit, if that is fine?

Same, same :D That is resolved than.


fn test_kill_zero_self_should_succeed() !void {
try posix.kill(posix.getpid(), .INVAL);
}
Loading