Skip to content

Clean up numeric overflow using u128 for product of u64 and 1_000 - #1113

Closed
worikgh wants to merge 1 commit into
RustAudio:masterfrom
worikgh:numeric_overflow_micros_to_stream_instant
Closed

Clean up numeric overflow using u128 for product of u64 and 1_000#1113
worikgh wants to merge 1 commit into
RustAudio:masterfrom
worikgh:numeric_overflow_micros_to_stream_instant

Conversation

@worikgh

@worikgh worikgh commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

I some cases I observed in host/jack/stream.rs the function micros_to_stream_instant the parameter micros when multiplied by 1_000 overflowed u64

@worikgh worikgh mentioned this pull request Mar 1, 2026
Comment thread src/host/jack/stream.rs

fn micros_to_stream_instant(micros: u64) -> crate::StreamInstant {
let nanos = micros * 1000;
let nanos = micros as u128 * 1000;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch. This can be even simpler:

fn micros_to_stream_instant(micros: u64) -> crate::StreamInstant {
    crate::StreamInstant::from_nanos_i128(micros as i128 * 1_000)
        .expect("`micros` out of range of `StreamInstant` representation")
}

And it seems the same issue is present in the ASIO and CoreAudio hosts:

src/host/coreaudio/mod.rs:

-    let nanos = m_host_time * info.numer as u64 / info.denom as u64;
-    let secs = nanos / 1_000_000_000;
-    let subsec_nanos = nanos - secs * 1_000_000_000;
-    Ok(crate::StreamInstant::new(secs as i64, subsec_nanos as u32))
+    let nanos = m_host_time as u128 * info.numer as u128 / info.denom as u128;
+    crate::StreamInstant::from_nanos_i128(nanos as i128).ok_or(BackendSpecificError {
+        description: "host time out of range of `StreamInstant` representation".to_string(),
+    })

src/host/asio/stream.rs:

-    let systime_ns = asio_ns_to_double(system_time);
-    let secs = systime_ns as i64 / 1_000_000_000;
-    let nanos = (systime_ns as i64 - secs * 1_000_000_000) as u32;
-    crate::StreamInstant::new(secs, nanos)
+    let nanos = (system_time.hi as u64) << 32 | system_time.lo as u64;
+    crate::StreamInstant::from_nanos_i128(nanos as i128)
+        .expect("`system_time` out of range of `StreamInstant` representation")

Would you be so kind to update those too and add appropriate changelog entries?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Would you be so kind to update those too and add appropriate changelog entries?

Forgive me: Are you asking me to update my pull request? I will. I am inexperienced at doing so, I will do my best.

Will not do any harm if you were not addressing me - you can ignore it.

Soon....

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Indeed if you would want to? No worries about the experience, good that you call that out. Let me know where I can help out.

@worikgh worikgh closed this Mar 1, 2026
@worikgh
worikgh deleted the numeric_overflow_micros_to_stream_instant branch March 1, 2026 21:52
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