Skip to content

[WIP] der: remove lifetime from OctetStringRef #1998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tarcieri
Copy link
Member

Following the pattern of #1921, removes the lifetime from the struct, instead changing OctetStringRef to a proper reference type to be used as &OctetStringRef.

This makes it possible to impl Borrow<OctetStringRef> for OctetString and impl ToOwned for OctetStringRef, so they can work with Cow.

cc @dishmaker

@tarcieri tarcieri requested a review from baloo August 18, 2025 14:22
@tarcieri
Copy link
Member Author

Custom derive needs to be updated to be aware that certain types are reference types and need to be used as e.g. <&OctetStringRef>::decode instead of OctetStringRef::decode

Following the pattern of #1921, removes the lifetime from the struct,
instead changing `OctetStringRef` to a proper reference type to be used
as `&OctetStringRef`.

This makes it possible to `impl Borrow<OctetStringRef> for OctetString`
and `impl ToOwned for OctetStringRef`, so they can work with `Cow`.
@tarcieri tarcieri force-pushed the der/remove-octet-string-ref-lifetime branch from 8cee1e0 to 2359ca5 Compare August 18, 2025 14:38
Comment on lines +86 to 91
impl FixedTag for OctetStringRef {
const TAG: Tag = Tag::OctetString;
}

impl OrdIsValueOrd for OctetStringRef<'_> {}

impl<'a> From<&OctetStringRef<'a>> for OctetStringRef<'a> {
fn from(value: &OctetStringRef<'a>) -> OctetStringRef<'a> {
*value
}
impl FixedTag for &OctetStringRef {
const TAG: Tag = Tag::OctetString;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a little weird, but seems to work

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