Skip to content
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

Cpak (mpak) format fix #7

Closed
wants to merge 13 commits into from

Conversation

networkfusion
Copy link

@networkfusion networkfusion commented Jun 25, 2021

Here is an attempted fix for correctly formatting the controller (memory) PAK correctly. It is based on the work done with bryc for use in Krikzz OS.

I am sure it could be optimized and willing to improve it with help (or you are welcome to improve it regardless) to conform to libdragon coding practices.

Resources:

to correctly generate unique serial number and full initialization.
@networkfusion networkfusion changed the base branch from master to trunk June 25, 2021 16:18
@networkfusion networkfusion marked this pull request as ready for review June 25, 2021 17:26
@bryc
Copy link

bryc commented Apr 17, 2022

In 2014 I brought up issue DragonMinded#13, about validating the header checksum properly, and was seemingly addressed in 2017-2019, so I closed the issue. I then made DragonMinded#105, which concerns the format_mempak function. In short, it's doing a bad job of reformatting the entire pak. It's copying static data from an emulator and probably doing this loop slightly wrong (the first few words should be blank).

Looking a bit closer at validate_mempak, it seems it can cause unnecessary data loss, because it will erase all data if just one checksum is incorrect instead of trying to use the backup checksums. Basically this is not robust and should be looked at. As a basic improvement, my suggestion would be to follow the general behaviour of Libultra: only erase everything if ALL checksums fail. And when reformatting, do not use predefined data obtained from emulator, instead generate new random serial and recalculate checksum.

My understanding of networkfusion's code is that it is a good and efficient method of erasing and reformatting the data. For his use case there was no intention to repair existing data - it was an option to wipe your pak completely. It would be important to determine if that is what libdragon needs, or if libdragon should have a more robust solution to repair existing data.

A more robust mempak library can parse the filesystem and selectively apply repair operations as transparently as possible. This is of course more work, but I already have my own very accurate library, almost as robust as the official one, I'm just missing some complicated filesystem repair techniques. Its a JavaScript project, but probably not too hard to rebase in C. Code here: https://github.com/bryc/mempak/blob/master/js/parser.js

Btw, that link to the serial database is old, this is the new one: https://rentry.co/mpk_serials 😃

@ottelo9
Copy link

ottelo9 commented Jan 6, 2023

Hi. It would be nice if you could merge it into the main branch! Because actually I have problems with the mempack functionality in ALT64 for ED64P.

DragonMinded#326

@ottelo9
Copy link

ottelo9 commented Jan 8, 2023

@networkfusion
Do you know why @rasky doesnt merge it? And why don't you have the change inside your libdragon fork?

@rasky
Copy link
Owner

rasky commented Jan 8, 2023

I know almost nothing of mempak. To merge this, I would need some study first. Moreover the PR as-is contains commented code blocks, and doesn’t add new tests (admittedly, the testsuite doesn’t test the mempak yet so somebody would have first to figure it out how to write tests for mempak.c in the first place). I guess I’ll get to this, eventually, like I did with the other PRs. Meanwhile any testing is appreciated

@ottelo9
Copy link

ottelo9 commented Jan 8, 2023

@rasky
Ok thanks for your answer. I would happy to test it but I use the github actions in my fork to build Altra64 binary for my ED64P. I dont know how to include this code into the existing docker build with github actions. Do you know how to do this? This is my Fork: https://github.com/ottelo9/altra64

@bryc
Copy link

bryc commented Jan 8, 2023

@ottelo9 This commit would only improve format_mempak(), and not fix any parsing problem in validate_mempak(). Different fixes may be needed.

@networkfusion networkfusion marked this pull request as draft February 4, 2023 20:36
@networkfusion
Copy link
Author

@networkfusion Do you know why @rasky doesnt merge it? And why don't you have the change inside your libdragon fork?

It is still WiP. But... hopefully the latest changes will help.

You can download the latest test ROM artifacts and see if they work... I "would" update Altra64, but it is not my priority at the moment...

@networkfusion
Copy link
Author

Closed as fixed in upsteam unstable.

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.

4 participants