Use named fields on UnableToCreateSnapshotFromSource; escape Raw paths in Display#802
Open
demoray wants to merge 2 commits into
Open
Use named fields on UnableToCreateSnapshotFromSource; escape Raw paths in Display#802demoray wants to merge 2 commits into
demoray wants to merge 2 commits into
Conversation
…paths
Two surgical changes in src/snapshot.rs:
1. `UnableToCreateSnapshotFromSource(Box<Error>, Source)` was the
only positional-tuple variant in the codebase — Error::Io,
Error::WriteBlock, Error::DiskUsageEstimateExceeded, and
Error::Other all use named fields. Switch this one to
`{ src, source }` for consistency and so the construction site
self-documents.
2. `Source::Raw`'s Display impl wrote `path.display()`, which lets
control characters, ANSI escapes, and embedded newlines flow
verbatim into error messages that may be logged or shown in CI
annotations. Use `{path:?}` instead — PathBuf's Debug quotes and
escapes.
Updated the one internal pattern match (the disk-usage predicate
one construction site (`create_source`).
Public API surface unchanged in shape — Error is a public enum, so
the variant rename of its fields is visible to any external
matcher, but the only documented downstream is the codebase's own
custom Debug impl which uses Display.
Verified with:
- cargo fmt --check
- cargo clippy --all-features --all-targets -- -D warnings
- cargo test --all-features
…ng)]
The pedantic lint flags {path:?} on PathBuf and suggests
path.display(), but escaping is the entire reason for this change.
Add a comment explaining the security rationale and an #[expect]
attribute with that reason so a future suppression of the lint will
fire a warning instead of silently going away.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Two surgical changes in
src/snapshot.rs.1. Named fields on
UnableToCreateSnapshotFromSourceIt was the only positional-tuple variant in this error enum —
Error::Io,Error::WriteBlock,Error::DiskUsageEstimateExceeded,and
Error::Otherall use named fields. The construction site increate_sourceis now self-documenting:2. Escape
Source::Rawpaths inDisplaypath.display()is lossy-but-prints-anything — control characters,ANSI escapes, embedded newlines all flow verbatim into error messages
that may be logged or rendered in CI annotations.
PathBuf'sDebugquotes and escapes, which is the right default inside a structured
error.
Scope and compatibility
try_method!macro; Replace try_method! macro with an explicit predicate and inline matches #801 deletes the macro entirely but this PRstands alone either way) and the one construction site.
Errorispub, so the variant rename is technically visible toany external
matcharm. The only documented downstream is thiscrate's own custom
Debugimpl, which goes throughDisplay.Not included
The larger restructure I described — splitting
Sourceinto aKernelSourceenum (the three well-known kernel devices, fullyValueEnum-exposed) plus a separateSnapshot::raw_source(path)builder method — is a public-API change worth raising as an issue
first.
Source::Rawis currently#[value(skip)], so it's library-only and doesn't participate in the auto-detect fallback; that split
would tighten the type but break any external library consumer.
Verified with:
cargo fmt --checkcargo clippy --all-features --all-targets -- -D warningscargo test --all-features