-
Notifications
You must be signed in to change notification settings - Fork 706
AMD-specific structures parsing #443
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
|
Traveling now, will be able to review in 3 weeks, so it will have to sit here for a while. |
vit9696
left a comment
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.
Before starting to review this more precisely, I would like
- to move unrelated changes to a separate PR
- to decide whether to use Kaitai for structure parsing
common/ffsparser.cpp
Outdated
| } | ||
|
|
||
| // Fill in table specific details | ||
| UINT32 checksumStart; |
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.
A lot of code here performs manual parsing. Why do not you use Kaitai?
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.
I don't know Kaitai. Was reading something but not enough - I have no time for it now.
|
PR is updated, mostly to get rid of all unrelated changes. @vit9696, please, check. |
…s), fixed nested unions/structures declarations, fixed crc validation and reporting
…r internal changes
|
Just returned from a surgery, will take me about a month to recover from it, hope to return to UEFITool development after that. I will review the changes here when capable. |
|
Tested locally on multiple AMD images, looks really nice and promising, thanks a lot. Merged. |
|
Thanks a lot for the work being done here, really appreciate it, These changes haven't been released yet into a new release (last release was made June 16 and may not be using the Just would like to test them real quick too, I could probably just build from the HEAD commit on the |
|
I'm planning a release soon, but meanwhile you can build the last commit, as you suggested, and test that way. |
|
@NikolajSchlej For example, the relationships between structures aren’t always clear - especially in complex images like for the ASUS ROG STRIX X570, where there are so many interconnections that it becomes hard to understand “who belongs to whom.” However, there’s one remaining issue I’m still working on: file attributes. The problem is that file attributes are stored in the table entries, not in the files themselves. But a single file can have multiple parent table entries, which might specify conflicting attributes at the same time. Showing attributes in the table entry info makes sense - but hiding them in the file info feels uninformative and ends up being quite inconvenient in practice. So I’m currently figuring out the best way to handle this. Would you be okay giving me a bit more time to sort this out? 😊 |
|
Sure. As for conflicting attributes, check if that is really the case, and show them both in the table (where they can't conflict) and in the file info (where instead of just "Attributes" for non-conflicting case there will be "Attributes (Bank0)" and "Attributes (Bank1)") or something alike. |
Tested compiling and working on Windows, Ubuntu 25.04, macOS Sequoia.
TODO:
This PR is based on: