Skip to content

[CString panic reduction] Reduce panics without changing return types - #1308

Open
DanielEScherzer wants to merge 28 commits into
rust-lang:mainfrom
DanielEScherzer:cstring-panics
Open

[CString panic reduction] Reduce panics without changing return types#1308
DanielEScherzer wants to merge 28 commits into
rust-lang:mainfrom
DanielEScherzer:cstring-panics

Conversation

@DanielEScherzer

Copy link
Copy Markdown
Contributor

For functions where the failure can be effectively community via the existing return type, instead of panicking when a CString cannot be created, return false or None or an error variant.

Part of #1297

…str()`

Add a `#[should_panic]` regression test for `ObjectType::from_str()` with an
invalid string.
Instead of panicking, treat failure to create a `CString` as indicating that
the provided string does not correspond to valid object type.
…hes()`

Add a `#[should_panic]` regression test for `Refspec::dst_matches()` with an
invalid string.
Instead of panicking, treat failure to create a `CString` as indicating that
the provided string is not matched by the refspec.
…hes()`

Add a `#[should_panic]` regression test for `Refspec::src_matches()` with an
invalid string.
Instead of panicking, treat failure to create a `CString` as indicating that
the provided string is not matched by the refspec.
…m()`

Add a `#[should_panic]` regression test for `Refspec::transform()` with an
invalid string.
Instead of panicking, convert the `alloc::ffi::NulError` error from the failure
to create a `CString` into a git2 `Error` object and return an error variant.
…rm()`

Add a `#[should_panic]` regression test for `Refspec::rtransform()` with an
invalid string.
Instead of panicking, convert the `alloc::ffi::NulError` error from the failure
to create a `CString` into a git2 `Error` object and return an error variant.
…_ref()`

Add a `#[should_panic]` regression test for `Transaction::lock_ref()` with an
invalid string.
Instead of panicking, convert the `alloc::ffi::NulError` error from the failure
to create a `CString` into a git2 `Error` object and return an error variant.
…target()`

Add a `#[should_panic]` regression test for `Transaction::set_target()` with an
invalid string for the `refname` parameter.
For the `refname` parameter, instead of panicking, convert the
`alloc::ffi::NulError` error from the failure to create a `CString` into a git2
`Error` object and return an error variant.
…:set_target()`

Add a `#[should_panic]` regression test for `Transaction::set_target()` with an
invalid string for the `reflog_message` parameter.
…get()`

For the `reflog_message` parameter, instead of panicking, convert the
`alloc::ffi::NulError` error from the failure to create a `CString` into a git2
`Error` object and return an error variant.
…symbolic_target()`

Add a `#[should_panic]` regression test for
`Transaction::set_symbolic_target()` with an invalid string for the `refname`
parameter.
…_target()`

For the `refname` parameter, instead of panicking, convert the
`alloc::ffi::NulError` error from the failure to create a `CString` into a git2
`Error` object and return an error variant.
…:set_symbolic_target()`

Add a `#[should_panic]` regression test for
`Transaction::set_symbolic_target()` with an invalid string for the `target`
parameter.
…bolic_target()`

For the `target` parameter, instead of panicking, convert the
`alloc::ffi::NulError` error from the failure to create a `CString` into a git2
`Error` object and return an error variant.
…:set_symbolic_target()`

Add a `#[should_panic]` regression test for
`Transaction::set_symbolic_target()` with an invalid string for the
`reflog_message` parameter.
…bolic_target()`

For the `reflog_message` parameter, instead of panicking, convert the
`alloc::ffi::NulError` error from the failure to create a `CString` into a git2
`Error` object and return an error variant.
…reflog()`

Add a `#[should_panic]` regression test for `Transaction::set_reflog()` with an
invalid string.
Instead of panicking, convert the `alloc::ffi::NulError` error from the failure
to create a `CString` into a git2 `Error` object and return an error variant.
…ve()`

Add a `#[should_panic]` regression test for `Transaction::remove()` with an
invalid string.
Instead of panicking, convert the `alloc::ffi::NulError` error from the failure
to create a `CString` into a git2 `Error` object and return an error variant.
…tes()`

Add a `#[should_panic]` regression test for `Tree::get_name_bytes()` with an
invalid string.
Instead of panicking, convert the `alloc::ffi::NulError` error from the failure
to create a `CString` into a git2 `Error` object and return an error variant.

@teor2345 teor2345 left a comment

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.

These all seem sensible to me, it's a routine and somewhat mechanical change.

(I don't have merge permissions here, but sometimes reviewers that do appreciate knowing others have checked the PR.)

View changes since this review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Waiting on review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants