Skip to content

fix: unsafe unwrap usage in into_string() conversion#388

Closed
famouswizard wants to merge 1 commit intonexus-xyz:mainfrom
famouswizard:patch-1
Closed

fix: unsafe unwrap usage in into_string() conversion#388
famouswizard wants to merge 1 commit intonexus-xyz:mainfrom
famouswizard:patch-1

Conversation

@famouswizard
Copy link

Describe your changes.

i replaced the unsafe unwrap() call in the into_string() conversion with a safer error handling approach.

this prevents potential panics when the conversion fails and ensures the program behaves more reliably.

@github-actions
Copy link

github-actions bot commented Apr 13, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@famouswizard
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

let mut dest = match std::env::var_os("OUT_DIR") {
Some(path) => path.into_string().unwrap(),
None => "/tmp/nexus-target".into(),
Some(path) => path.into_string().unwrap_or_else(|_| "/tmp/nexus-target".to_string()),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be nice to make this more robust, but if this path is invalidly set we should return a BuildError (rather than panicking), not fall back to the default.

chuckyLeeVIII pushed a commit to chuckyLeeVIII/nexus-zkvm that referenced this pull request Apr 18, 2025
Summary:

Fixes nexus-xyz#386

Test Plan:

Co-authored-by: duc-nx <>
sjudson pushed a commit that referenced this pull request May 14, 2025
Summary:

Fixes #386

Test Plan:

Co-authored-by: duc-nx <>
@sjudson
Copy link
Contributor

sjudson commented Oct 24, 2025

Closing due to inactivity...

@sjudson sjudson closed this Oct 24, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Oct 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants