Avoid allocating in pre_exec closure
#340
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The comment
// The closure now ONLY contains async-signal-safe syscall wrappers.is misleading, since allocations are not async-signal-safe. There are three sources of allocation:std::io::Error::other(see Document Error::{new,other} as to be avoided in pre_exec rust-lang/rust#148971)format!CString::newI've moved out
CString::newsimilarly to other strings, hopefully that's idiomatic for your project.Error::otherandformat!are trickier. Error messages are not transferred acrosspre_execboundary, so whatever error messages you wrote were never visible. I replaced error handling with?, which uses the non-allocatingimpl From<nix::Error> for std::io::Error, under assumption that if error messages weren't transmitted before, it's fine to ignore them now.Another possible alternative would be to use
.expect, effectively transmitting the error via stderr. I couldn't make a judgement on whether that's correct for use case, so I refrained from doing that.