8000 Isolated Processes 15: Modify API to use serializable types by marti4d · Pull Request #203 · rust-minidump/minidump-writer · GitHub
[go: up one dir, main page]

Skip to content

Isolated Processes 15: Modify API to use serializable types - #203

Merged
gabrielesvelto merged 2 commits into
mainfrom
isolated_process/15-serializable-types
Aug 25, 2026
Merged

gabrielesvelto merged 2 commits into
mainfrom
isolated_process/15-serializable-types

Conversation

@marti4d
@marti4d marti4d commented Aug 10, 2026
Copy link
Copy Markdown
Collaborator

No description provided.

@marti4d
marti4d force-pushed the isolated_process/14-process-backend-traits branch from 2f0b9f3 to 06e3ae5 Compare August 11, 2026 17:29
@Jake-Shadle
Jake-Shadle removed their request for review August 12, 2026 08:21
@marti4d
marti4d force-pushed the isolated_process/15-serializable-types branch from 3dd831e to 68bf045 Compare August 12, 2026 18:26
@marti4d
marti4d commented Aug 12, 2026 •
Copy link
Copy Markdown
Collaborator Author

If you are curious about the motivation here, read the top of the src/regs.rs file, but basically: libc is rather inconsistent about defining a lot of the constants/structs for the different register sets, it sometimes doesn't define them even though the kernel technically supports them, and it varies depending on whether you're using glibc, bionic, or musl.

They also don't implement Default, Deserialize or Serialize, even though they all could because zero would be a reasonable default for all fields, and they could all easy implement the serde traits.

So... I basically went through the kernel, duplicated them myself into the regs.rs file, and cited the actual kernel source code for each structure.

The nice thing, now, is that I have a new unsafe trait PtraceRegisterSet { ... }, that must ensure that the ptrace note, request, kernel size, and output type all match each other. That allows the API calls themselves to be made safe for any of the types that I've manually checked against the kernel code.

@marti4d
marti4d force-pushed the isolated_process/14-process-backend-traits branch from 06e3ae5 to 3358ffc Compare August 17, 2026 15:15
@marti4d
marti4d force-pushed the isolated_process/15-serializable-types branch from 68bf045 to 633d4b0 Compare August 17, 2026 15:16
@schopin-mozilla schopin-mozilla left a comment
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Altogether LGTM, just a few suggestions to improve readability.

@marti4d
marti4d force-pushed the isolated_process/14-process-backend-traits branch from 3358ffc to f5a3d4b Compare August 20, 2026 14:37
@marti4d
marti4d force-pushed the isolated_process/15-serializable-types branch from 633d4b0 to ff99719 Compare August 20, 2026 18:45
@marti4d
marti4d force-pushed the isolated_process/14-process-backend-traits branch from f5a3d4b to efd58e0 Compare August 20, 2026 19:30
@marti4d
marti4d force-pushed the isolated_process/15-serializable-types branch from ff99719 to fcf9e47 Compare August 20, 2026 19:30
Base automatically changed from isolated_process/14-process-backend-traits to main August 21, 2026 19:49
@gabrielesvelto gabrielesvelto left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, this is such a big improvement in terms of readability and robustness over what we had and a huge leap over the breakpad code we're coming from.

@gabrielesvelto
gabrielesvelto merged commit 68cf65d into main Aug 25, 2026
16 checks passed
@marti4d
marti4d deleted the isolated_process/15-serializable-types branch August 25, 2026 14:10
@marti4d
marti4d commented Aug 25, 2026
Copy link
Copy Markdown
Collaborator Author

LGTM, this is such a big improvement in terms of readability and robustness over what we had and a huge leap over the breakpad code we're coming from.

Thanks @gabrielesvelto - That's kind of you to say 😊

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.

3 participants

0