Consolidate CMSG_* implementations #4908

gibbz00 · 2026-01-03T13:32:53Z

~~Builds upon #4903~~Merged

The original intent was to simplify the reasoning for how they differ across platforms. It should as a consequence also make it easier to extend target support.

Changes:

CMSG_* functions which can be const are all marked const -- consistently ¹.
Removes unsafe from CMSG_ALIGN, CMSG_SPACE, CMSG_LEN.

Fixes:

Custom CMSG_FIRSTHDR implementation for VxWorks had missed to check that mhdr.msg_controllen >= size_of::<cmsghdr>(), as per POSIX 1003.1-2024.

Documents:

Usage and safety requirements.
Adds missing references to upstream headers.

Breaking API change Suggestions

Perhaps CMSG_FIRSTHDR can take mhdr by reference? First thing being done is otherwise a raw pointer deref under practically no safety guarantees. Doing so would also remove the need to mark it unsafe.
Same reasoning can be applied to mhdr in CMSG_NXTHDR, difference being that the function must remain unsafe for dereferencing cmsg.

Except CMSG_DATA on solarish. It does align(cmsg_addr + size_of(cmsg)) rather than cmsg_addr + align(size_of(cmsg>)). The others can get away with not casting the cmsg pointer to usize by instead casting cmsg to a byte pointer and doing the addition using byte_ptr.offset(). ↩

rustbot · 2026-01-03T13:32:56Z

Some changes occurred in the Android module

cc @maurer

Some changes occurred in solarish module

cc @jclulow, @pfmooney

Some changes occurred in OpenBSD module

cc @semarie

tgross35

Thank you for going through the work here! I need to do a more thorough read-through, but have a few initial comments.

tgross35 · 2026-01-05T12:18:43Z

src/new/apple/libc/mod.rs

Is this one actually getting picked up? The module is defined here

libc/src/new/apple/mod.rs

Lines 9 to 12 in ab195eb

mod libc {

pub(crate) mod signal;

pub(crate) mod unistd;

}

There's a pub(crate) use libc::*; a bit further down :)

Hah, see what you mean. I seem to have made it into a mod.rs whilst keeping the mod libc {} somehow. I probably did when adding a libc/sys/socket.rs, realizing later on that it should be under xnu/, (hence the out-of-place freebsd copy-pasta).

Anyhow, I've now removed the libc/sys/socket.rs, making the mod.rs creation irrelevant to this PR... Do you want med to keep it anyway?

src/posix.rs

src/new/apple/libc/sys/socket.rs

src/new/common/posix/sys/socket.rs

tgross35 · 2026-01-05T20:31:34Z

src/new/common/posix/sys/socket.rs

+// HACK: AIX does not use any alignment/padding for ancillary data. Setting
+// this to 1 it makes possible to reuse the CMSG_* implementatinos as the extra
+// CMSG_ALIGN calls become no-op.
+#[cfg(target_os = "aix")]
+pub(crate) type __ALIGN_BOUNDARY = u8;
+
+#[cfg(target_os = "android")]
+pub(crate) type __ALIGN_BOUNDARY = usize;
+
+#[cfg(target_vendor = "apple")]
+pub(crate) type __ALIGN_BOUNDARY = u32;
+
+#[cfg(target_os = "hurd")]
+pub(crate) type __ALIGN_BOUNDARY = usize;
+
+#[cfg(target_os = "cygwin")]
+pub(crate) type __ALIGN_BOUNDARY = usize;
+
+#[cfg(target_os = "dragonfly")]
+pub(crate) type __ALIGN_BOUNDARY = c_long;
+
+#[cfg(target_os = "emscripten")]
+pub(crate) type __ALIGN_BOUNDARY = usize;
+
+#[cfg(target_os = "fuchsia")]
+pub(crate) type __ALIGN_BOUNDARY = c_long;
+
+#[cfg(target_os = "haiku")]
+pub(crate) type __ALIGN_BOUNDARY = usize;
+
+#[cfg(target_os = "l4re")]
+pub(crate) type __ALIGN_BOUNDARY = usize;
+
+#[cfg(target_os = "linux")]
+pub(crate) type __ALIGN_BOUNDARY = usize;
+
+#[cfg(target_os = "nto")]
+pub(crate) type __ALIGN_BOUNDARY = usize;
+
+#[cfg(target_os = "redox")]
+pub(crate) type __ALIGN_BOUNDARY = size_t;
+
+#[cfg(target_os = "vxworks")]
+pub(crate) type __ALIGN_BOUNDARY = usize;
+
+cfg_if! {
+    if #[cfg(any(target_os = "illumos", target_os = "solaris"))] {
+        #[cfg(target_arch = "sparc64")]
+        pub(crate) type __ALIGN_BOUNDARY = u64;
+        #[cfg(not(target_arch = "sparc64"))]
+        pub(crate) type __ALIGN_BOUNDARY = u32;
+    }
+}
+
+cfg_if! {
+    if #[cfg(target_os = "netbsd")] {
+        cfg_if! {
+            if #[cfg(target_arch = "x86")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_int;
+            } else if #[cfg(target_arch = "x86_64")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_long;
+            } else if #[cfg(target_arch = "aarch64")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_int;
+            } else if #[cfg(target_arch = "arm")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_longlong;
+            } else if #[cfg(target_arch = "mips")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_longlong;
+            } else if #[cfg(target_arch = "powerpc")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_double;
+            } else if #[cfg(target_arch = "riscv64")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_long;
+            } else if #[cfg(target_arch = "sparc64")] {
+                pub(crate) type __ALIGN_BOUNDARY = u128;
+            }
+        }
+    }
+}
+
+cfg_if! {
+    if #[cfg(target_os = "freebsd")] {
+        cfg_if! {
+            if #[cfg(target_arch = "x86")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_long;
+            } else if #[cfg(target_arch = "x86_64")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_long;
+            } else if #[cfg(target_arch = "aarch64")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_longlong;
+            } else if #[cfg(target_arch = "arm")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_int;
+            } else if #[cfg(target_arch = "powerpc")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_int;
+            } else if #[cfg(target_arch = "powerpc64")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_long;
+            } else if #[cfg(target_arch = "riscv64")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_longlong;
+            }
+        }
+    }
+}
+
+cfg_if! {
+    if #[cfg(target_os = "openbsd")] {
+        cfg_if! {
+            if #[cfg(target_arch = "x86")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_int;
+            } else if #[cfg(target_arch = "x86_64")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_long;
+            } else if #[cfg(target_arch = "aarch64")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_long;
+            } else if #[cfg(target_arch = "arm")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_double;
+            } else if #[cfg(target_arch = "mips64")] {
+                pub(crate) type __ALIGN_BOUNDARY = u64;
+            } else if #[cfg(target_arch = "powerpc")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_double;
+            } else if #[cfg(target_arch = "powerpc64")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_long;
+            } else if #[cfg(target_arch = "riscv64")] {
+                pub(crate) type __ALIGN_BOUNDARY = c_long;
+            } else if #[cfg(target_arch = "sparc64")] {
+                pub(crate) type __ALIGN_BOUNDARY = u128;
+            }
+        }
+    }
+}


We're trying to move away from this kind of nested config in the new structure. Could you move them to the respective socket modules instead?

Also, this is something used so the shared impl is possible rather than actually being in source, right? A comment to that effect would be helpful.

Where do these definitions come from by the way? Looking at e.g. NetBSD I see https://github.com/NetBSD/src/blob/e46385ea0f041fae85cfb2a10b8b4910124827b8/sys/sys/socket.h#L544-L548 with __ALIGNBYTES defined in a few different places https://github.com/search?q=repo%3ANetBSD%2Fsrc+%2F%23define+__ALIGNBYTES%2F&type=code, but nothing using types like double.

Yeah, I know :/ They were in their respective modules at first, (not the new/ ones), but then I started to get some lint warnings. Something about "declaration in module after use", can't remember which one exactly.

I left them here, expecting some discussion on what best to do. Should have been a bit clearer on that part.

They're technically all used solely for CMSG_ALIGN. Went ahead and removed pub(crate) to make that clearer.

Where do these definitions come from by the way?

Yeah, I also noticed a lot of inconsistencies with upstream, notably in what type is used for the size_of. Most are pretty harmless, Ex. Linux-like CMSG_ALIGN implementations normally use size_t or long, but this crate will often use usize.

Decided on not "correct" these. Wasn't keen on creating any unintended breaking changes. The one's I did change though were those that where previously defined as the result of size_of::<something>(), opting for the corresponding uN type alias. Solarish for example, had its alignment constant changed from const _CMSG_HDR_ALIGNMENT: usize = 8; to type __ALIGN_BOUNDARY = u64;.

So what do you think? Make an exception here to using nested config in new and add a source comment on why? I may otherwise be a bit confusing that common/posix/sys/socket.rs is importing a single use pub(crate) __ALIGN_BOUNDARY defined in a <target>/sys/socket.rs.

tgross35 · 2026-01-05T20:36:38Z

src/new/common/bsd.rs

+pub(crate) mod sys {
+    pub(crate) mod socket {


Could you make sys a new directory and socket a new module in it? I've been trying to avoid >1 layer of nesting in the new modules, and sys tends to have enough contents that it's worth its own directory.

(Applies to a few places)

Done ️✌️ Anything still missing?

tgross35 · 2026-01-05T20:43:40Z

src/new/common/posix/sys/socket.rs

+    // TODO: why not take mhdr by ref?
+    let msghdr = unsafe { *mhdr };
+
+    // SAFETY:
+    // - Caller ensured pointer is not null.
+    // - Pointer retrieved from either CMSG_FIRSTHDR or CMSG_NXTDR,
+    //   thus ensuring that is lies between mhdr_addr and mhdr_addr +
+    //   msg_controllen.
+    let cmsghdr = unsafe { *cmsg };


Did these come from the original impls?

I would slightly prefer reading the fields only as-needed, so as to avoid creating UB if the struct is partially initialized. IMO it's a terrible idea to ever use non-zeroed libc types but unfortunately people do it, so we should avoid giving them more unsoundness then they'd get from the same macro calls in C.

Agreed. Changed it to:

let cmsg_len = unsafe { *cmsg }.cmsg_len; // .. let mut max_addr = { // IMPROVEMENT: why not take mhdr by ref? let msghdr = unsafe { *mhdr }; msghdr.msg_control as usize + msghdr.msg_controllen as usize };

tgross35 · 2026-01-05T20:45:46Z

src/new/common/posix/sys/socket.rs

+#[cfg(not(any(
+    target_env = "musl",
+    target_env = "ohos",
+    target_os = "emscripten",
+    target_os = "fuchsia"
+)))]
+pub unsafe fn CMSG_NXTHDR(mhdr: *const msghdr, cmsg: *const cmsghdr) -> *mut cmsghdr {
+    unsafe { next_impl(mhdr, cmsg, true) }
+}


The BSDs define a CMSG_NXTHDR too right? Doesn't this wind up duplicated?

Wrote it in such a way that the BSDs reuse the CMSG_NXTHDR, something like;

// common/bsd pub unsafe fn CMSG_NXTHDR(mhdr: *const msghdr, cmsg: *const cmsghdr) -> *mut cmsghdr { if cmsg.is_null() { CMSG_FIRSTHDR(mhdr) } else { CMSG_NXTHDR(mhdr, cmsg) } }

It's actually the more POSIX compliant way:

If the first argument is a pointer to a msghdr structure and the second argument is a null pointer, this macro shall be equivalent to CMSG_FIRSTHDR(mhdr).

https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/sys_socket.h.html

tgross35 · 2026-01-05T21:08:36Z

src/new/common/posix/sys/socket.rs

+pub(crate) unsafe fn next_impl(
+    mhdr: *const msghdr,
+    cmsg: *const cmsghdr,
+    allow_zero_sized_payload: bool,
+) -> *mut cmsghdr {


Anything here that couldn't be made const at our MSRV? I think you may as well constify CMSG_NXTHDR if so, I could imagine it's occasionally useful.

It's the usize pointer casts, tried to look into it as well.

Transmuting pointers to integers in a const context is undefined behavior, unless the pointer was originally created from an integer.

https://doc.rust-lang.org/std/mem/fn.transmute.html#transmutation-between-pointers-and-integers

rustbot · 2026-01-05T21:09:17Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

The original intent was to simplify the reasoning for how they differ across platforms. It should as a consequence also make it easier to extend target support. Changes: - CMSG_* functions which can be const are all marked const. - Removes unsafe from `CMSG_ALIGN`, `CMSG_SPACE`, `CMSG_LEN`. Fixes: Custom CMSG_FIRSTHDR implementation for VxWorks had missed to check that `mhdr.msg_controllen >= size_of::<cmsghdr>()`, as per POSIX 1003.1-2024. Documents: - Usage and safety requirements. - Adds missing references to upstream headers.

rustbot · 2026-01-06T12:24:07Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

gibbz00 · 2026-01-06T13:10:42Z

@rustbot ready

rustbot added O-android O-arm O-bsd O-dragonfly O-linux O-linux-like O-macos O-mips O-powerpc O-redox O-riscv O-solarish O-sparc O-unix O-x86 S-waiting-on-review labels Jan 3, 2026

gibbz00 mentioned this pull request Jan 3, 2026

Cmsg len space size t #4905

Open

gibbz00 force-pushed the consolidate_cmsg_next branch from b15d3d5 to e9e4e18 Compare January 5, 2026 09:33

This comment has been minimized.

Sign in to view

tgross35 requested changes Jan 5, 2026

View reviewed changes

rustbot added S-waiting-on-author and removed S-waiting-on-review labels Jan 5, 2026

gibbz00 force-pushed the consolidate_cmsg_next branch from e9e4e18 to ab33767 Compare January 6, 2026 12:24

rustbot added S-waiting-on-review and removed S-waiting-on-author labels Jan 6, 2026

gibbz00 mentioned this pull request Jan 6, 2026

AIX: Only test addresses that are multiples of the size of cmsghdr #4916

Merged

3 tasks

Consolidate CMSG_* implementations #4908

Are you sure you want to change the base?

Consolidate CMSG_* implementations #4908

Uh oh!

Conversation

gibbz00 commented Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes:

Fixes:

Documents:

Breaking API change Suggestions

Footnotes

Uh oh!

rustbot commented Jan 3, 2026

Uh oh!

This comment has been minimized.

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jan 5, 2026

Uh oh!

rustbot commented Jan 6, 2026

Uh oh!

gibbz00 commented Jan 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gibbz00 commented Jan 3, 2026 •

edited

Loading