Skip to content

Conversation

@tarcieri
Copy link
Member

Right now PemReader uses interior mutability via RefCell, because peeking might involve filling an internal peek buffer (i.e. decoding the Base64 in a PEM document).

Rather than trying to hide the fact we're potentially modifying internal buffers (but not the cursor/position) when peeking, this changes the methods to explicitly accept &mut self, and with that change eliminates all usages of RefCell.

This change appears to have no practical drawbacks, since peeking always occurs during the decoding process where we need mutable access to the reader anyway.

Right now `PemReader` uses interior mutability via `RefCell`, because
peeking might involve filling an internal peek buffer (i.e. decoding the
Base64 in a PEM document).

Rather than trying to hide the fact we're potentially modifying internal
buffers (but not the cursor/position) when peeking, this changes the
methods to explicitly accept `&mut self`, and with that change
eliminates all usages of `RefCell`.

This change appears to have no practical drawbacks, since peeking always
occurs during the decoding process where we need mutable access to the
reader anyway.
@tarcieri tarcieri requested a review from baloo May 26, 2024 19:58
@tarcieri tarcieri merged commit 42ec503 into master May 26, 2024
@tarcieri tarcieri deleted the der/mut-self-peek-methods branch May 26, 2024 20:54
@tarcieri tarcieri mentioned this pull request Aug 17, 2024
tarcieri added a commit that referenced this pull request Aug 17, 2024
tarcieri added a commit that referenced this pull request Aug 17, 2024
tarcieri added a commit that referenced this pull request Aug 17, 2024
Removes the internal buffer added in #839 in favor of just cloning the
`PemReader`'s state.

This is a partial step towards #1475 and would greatly simplify the
implementation of #1474.

Also reverts: "der: have `Reader::peek*` methods take `&mut self` (#1418)"

This reverts commit 42ec503.
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