search

Alcaro / Flips

Floating IPS is a patcher for IPS and BPS files.
Other
334 stars 46 forks source link

BPS source checksum is for whole file (w/o header) but source length is less #58

Closed YoshiRulz closed 1 year ago

YoshiRulz commented 1 year ago

Spec is ambiguous on this but it feels like a bug. IMO the length is what should be changed.

Repro: Create a .bps patch using the headered US-region SMW rom SHA1:553CF42F35ACF63028A369608742BB5B913C103F as base. It's 0x80200 bytes, the .bps says source is 0x7FF80 bytes, but the embedded checksum is for the full thing sans header (i.e. the last 0x80000 bytes). I used version unstable-2021-10-28 from Nixpkgs.

Alcaro commented 1 year ago

Spec doesn't permit ignoring the header at all, I made that part up myself. Ignoring the header should mean 100% ignoring the header, as if the patch was created from unheadered ROMs.

Can't reproduce. I tried all four combinations of headered and unheadered source and target ROM (where target has the bytes 12 34 56 78 inserted at offset 0 or 0x200, depending on if it's headered); the resulting patches are identical. (Though it does complain that the ROMs are backwards if source is headered and target isn't; fixed.)

YoshiRulz commented 1 year ago

Added a Nix overlay for fdd5c6e34285beef5b9be759c9b91390df486c66 and used --info --verbose, and the size is indeed 524288 (=0x80000), it's my implementation of the varint decoder that's wrong. Sorry about that.

Alcaro commented 1 year ago

--info exists in 2021-10-28 too and will tell the expected file sizes. But you want the #57 fix, so overlay sounds good to me.

And you did point me toward one bug, so don't worry about it.