-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
spg format + [tsconf_betadisc_flop.xml] updated games/demos [holub, tslabs] #11274
Conversation
Diff looks strange with others commits leaked into this PR after merge. |
This seems to have got mixed up a bit with release branch commits from last month showing up as part of it. Could you merge or rebase to make GitHub less confused? |
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.
Was the decompression code cribbed from somewhere? It seems to be somewhat lacking in error checking as well as having some odd characteristics.
u8 data; | ||
}; | ||
|
||
spg_v10* spg = (spg_v10*)snapdata; |
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.
You should check the size here.
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.
It's not possible because declared blocks array will always oversize real data. If that can cause problems I can separate blocks from struct and process them iteratively
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.
@cuavas sorry to bother... what is the preference here?
u8 *data = &spg->data; | ||
for (u8 i = 0; i < spg->blocks_num; i++) | ||
{ | ||
const u16 size = (spg->blocks[i].size512 + 1) * 512; | ||
const u8 page = spg->blocks[i].page; | ||
const u16 offs = (spg->blocks[i].address512) * 512; |
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.
You don’t seem to have checked the size against the number of blocks to ensure the snapshot isn’t truncated.
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.
It's possible to be trunketed. 256 is max allowed but snapshot can contain any less numbers of blocks.
The decompression code has been copied from https://github.com/tslabs/zx-evo/blame/master/pentevo/unreal/Unreal/depack.cpp with my permission to any modifications |
@cuavas can you please take another look? |
This PR still has multiple things in it that might be better separated:
|
Oh, I'm sorry for bothering. Tried to rebase it in order to revisit. Will close the PR and reopen when ready. |
spg format currently has no alternatives, because all existing don't support pagination with >128K RAM.