Skip to content

Use shared references for MemReader methods#157

Merged
gabrielesvelto merged 1 commit intorust-minidump:mainfrom
marti4d:shared_memreader
Sep 26, 2025
Merged

Use shared references for MemReader methods#157
gabrielesvelto merged 1 commit intorust-minidump:mainfrom
marti4d:shared_memreader

Conversation

@marti4d
Copy link
Collaborator

@marti4d marti4d commented Sep 9, 2025

OnceLock is perfect for situations like this, where a one-time memoization needs to be done and then the object will never mutate again.

Now we can use shared references for MemReader, which is nice because sometimes it can be awkward not being able to read two areas of the other process's memory concurrently (despite there being no reason it can't be done).

Copy link
Contributor

@gabrielesvelto gabrielesvelto left a comment

Choose a reason for hiding this comment

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

Excellent, just one nit

std::sync::OnceLock,
};

#[derive(Debug)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the Debug trait required or can it be removed? I wouldn't add it unless the structure is user-printable.

Copy link
Collaborator Author

@marti4d marti4d Sep 26, 2025

Choose a reason for hiding this comment

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

Yep -- This is needed to be able to use Result::expect() on the potential Err(Style) that might be returned by OnceLock::set(). Alternatively, I could use .ok().expect(...) on it to avoid this, but honestly this is an internal type anyway - What's the point of avoiding #[derive(Debug)] on it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

It was just to avoid unnecessary codegen.

@gabrielesvelto gabrielesvelto merged commit ad67847 into rust-minidump:main Sep 26, 2025
16 checks passed
@marti4d marti4d deleted the shared_memreader branch September 26, 2025 18:07
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