Skip to content

Experimental autodetection of 64 bit timespec on 32 bit platforms #18

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
diwic wants to merge 2 commits into from
Closed

Experimental autodetection of 64 bit timespec on 32 bit platforms #18

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
235b0d6
Experimental autodetection of 64 bit timespec on 32 bit platforms
diwic Jun 18, 2026
942a91b
timespec: another attempt
diwic Jun 23, 2026
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
28 changes: 27 additions & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,33 @@
#![allow(non_upper_case_globals)]
#![no_std]

use libc::{FILE, pid_t, pollfd, timespec, timeval};
use libc::{FILE, pid_t, pollfd, timeval};

#[cfg(target_pointer_width = "64")]
#[derive(Copy, Clone, Eq, PartialEq, Default)]

@roderickvd roderickvd Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Debug may be nice to implement.

@roderickvd roderickvd Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In your rewrite I think these were dropped altogether (because they can't be derived on the union). That creates an asymmetry with libc::timespec, which does derive them.

#[repr(C)]
pub struct timespec([i64; 2]);

#[cfg(target_pointer_width = "64")]
impl timespec {
#[inline(always)]
pub fn tv_sec(&self) -> i64 { self.0[0] }
#[inline(always)]
pub fn tv_nsec(&self) -> i64 { self.0[1] }
}

#[cfg(target_pointer_width = "32")]
#[derive(Copy, Clone, Eq, PartialEq, Default)]
#[repr(C)]

@roderickvd roderickvd Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also do #[repr(C, align(8))] to ensure it doesn't segfault when ALSA tries to write? Rust may not guarantee the same alignment.

@diwic diwic Jun 23, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good catch!

@roderickvd roderickvd Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And good idea to use those native-width i64s for alignment 😄

pub struct timespec([i32; 4]);

#[cfg(target_pointer_width = "32")]
impl timespec {
fn is64(&self) -> bool { self.0[2] != 0 || self.0[3] != 0 }
pub fn tv_sec(&self) -> i64 { self.0[0] as i64 }

@roderickvd roderickvd Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If is64 we should retain the upper half:

pub fn tv_sec(&self) -> i64 {
    if self.is64() {
        #[cfg(target_endian = "little")]
        ((self.0[1] as i64) << 32) | (self.0[0] as u32 as i64)
        #[cfg(target_endian = "big")]
        ((self.0[0] as i64) << 32) | (self.0[1] as u32 as i64)
    } else {
        self.0[0] as i64
    }
}

...I think 🤯 time problems are hard.

@diwic diwic Jun 23, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@roderickvd Yeah and there is an edge case where is64 would return false because tv_nsec actually is 0. This would fail on big endian :-(

pub fn tv_nsec(&self) -> i64 { if self.is64() { (self.0[2] + self.0[3]) as i64 } else { self.0[1] as i64 } }

@roderickvd roderickvd Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

self.0[2] + self.0[3] is a nice trick and works because either field is just padding. It's a trick though. For clarity how would you feel about gating on endianness?

@diwic diwic Jun 23, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@roderickvd Having thought about it, I think we should apply this fix on little endian only. Big endian is incredibly uncommon.

@roderickvd roderickvd Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As uncommon as it may be, wouldn't it be nice for alsa-sys to work regardless?

}


#[cfg(feature = "use-bindgen")]
include!(concat!(env!("OUT_DIR"), "/generated.rs"));
Expand Down