-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Refactor MachObjectFile signature blobs #116566
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
Conversation
Tagging subscribers to this area: @vitek-karas, @agocke |
- Use classes instead of structs for blobs - Create IBlob interface for blob writing - Remove MachObjectFile.CodeSignature - Create IMachFileAccess to abstract reading and writing to disk
095fc30
to
df505ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the handling of Mach-O signature blobs by replacing structs with classes and introducing a common IBlob interface, while also consolidating and removing redundant code files. Key changes include:
- Converting blob representations to classes with read/write methods.
- Introducing the IBlob interface and reorganizing blob types.
- Abstracting file access via the new IMachOFileAccess interface.
Reviewed Changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
FileUtils/EndianConversionExtensions.cs | Adds ulong conversion method for consistency. |
Enums/HashType.cs | Adds HashTypeExtensions for creating hash algorithms and obtaining hash sizes. |
Enums/CodeDirectorySpecialSlot.cs | Updates comment with a reference link. |
Enums/BlobMagic.cs | Adjusts ordering of enum members without altering underlying values. |
BinaryFormat/SymbolTableLoadCommand.cs | Renames the struct for clarity and adjusts default check accordingly. |
BinaryFormat/LinkEditLoadCommand.cs | Renames the struct and updates the constructor and default check. |
BinaryFormat/Blobs/SuperBlob.cs | Introduces SuperBlob for managing composite blobs and validates blob offsets. |
BinaryFormat/Blobs/SimpleBlob.cs | Implements a simple blob type for binary read/write operations. |
BinaryFormat/Blobs/RequirementsBlob.cs | Wraps a SuperBlob to represent an empty Requirements blob. |
BinaryFormat/Blobs/EmbeddedSignatureBlob.cs | Implements an EmbeddedSignature blob that aggregates sub-blobs. |
BinaryFormat/Blobs/CodeDirectoryBlob.cs | Implements a CodeDirectoryBlob with header parsing and blob writing logic. |
BinaryFormat/Blobs/CmsWrapperBlob.cs | Implements a CmsWrapperBlob using a SimpleBlob as inner storage. |
BinaryFormat/Blobs/BlobParser.cs | Provides logic to parse a blob based on its magic number. |
BinaryFormat/Blobs/BlobIndex.cs | Adds a constant for blob index size. |
AppHost/HostWriter.cs | Updates Mach-O file access to use the new IMachOFileAccess interface. |
src/installer/managed/Microsoft.NET.HostModel/MachO/BinaryFormat/Blobs/CodeDirectoryBlob.cs
Outdated
Show resolved
Hide resolved
…ate identical objects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks!
I'm going to go ahead and merge. @elinor-fung I'll apply any review feedback to the follow up PR. |
Decided to split up #115800 into a refactor PR with no behavioral changes and a separate PR with the behavior changes. Unfortunately this still ended up being pretty large, but much of the diff is moving code or forwarding method calls in a wrapper.
This PR makes the following changes