Skip to content

fix: use portable ioctl request codes#63

Merged
kylecarbs merged 1 commit into
coder:mainfrom
rymdbar:portable-ioctl
Jun 16, 2026
Merged

fix: use portable ioctl request codes#63
kylecarbs merged 1 commit into
coder:mainfrom
rymdbar:portable-ioctl

Conversation

@rymdbar

@rymdbar rymdbar commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

While Zig's std does not set these variants for darwin under std.os., they are available with the exact same values under std.c. Thus there seems to be no reason to not use the portable constants.

@rymdbar

rymdbar commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Pull request updated with integration test change.

Running zig build test, zig build test-integration and zig build test-all completes successfully without any output on FreeBSD 15.0-RELEASE-p10.

@rymdbar

rymdbar commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

I see the build error and am a bit embarrassed.

src/pty.zig:15:43: error: struct 'c.T__struct_22974' has no member named 'IOCSWINSZ'
    pub const IOCSWINSZ: c_ulong = std.c.T.IOCSWINSZ;

Turns out the Apple specific code was needed, precisely like the comment said. I was sloppy to assume that std.c.T… contained an independent set of definitions, when actually reading the source shows a slightly more interesting situation. A rookie mistake, I've never seen Zig before.

An updated branch has been pushed.

@kylecarbs

Copy link
Copy Markdown
Member

@rymdbar no biggie at all! Looking again now.

@rymdbar

rymdbar commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Oh. I forgot about the integration tests duplicating the constants, and didn't realize to run zigfmt. Will push an update in a few minutes.

@rymdbar

rymdbar commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

I'm having trouble with integration tests failing, even with the same commit which I experienced was working earlier. My train is arriving its station now, so I'll need to get back to it later.

@rymdbar

rymdbar commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Seems my problem with integration test was that I accidentally ran them inside another boo session. When running them in a plain terminal, they work. 🙂

@kylecarbs

Copy link
Copy Markdown
Member

@rymdbar heh ;p

Using the constants under the `std.c` prefix rather than under `std.os.`
works on more platforms. This change can make the compile fail with an
actual error rather than "unsupported OS" on obscure platforms, but it
should be safe to assume anyone building on such niche systems prefers
this.
@rymdbar

rymdbar commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Repushed unchanged, after being rebased onto latest main.

@kylecarbs

Copy link
Copy Markdown
Member

Once CI passes I'll merge!

@kylecarbs kylecarbs merged commit 9170bfd into coder:main Jun 16, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants