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

USBExtreme format extended: #485

Merged
merged 2 commits into from Aug 17, 2021
Merged

USBExtreme format extended: #485

merged 2 commits into from Aug 17, 2021

Conversation

AKuHAK
Copy link
Member

@AKuHAK AKuHAK commented Aug 11, 2021

-added 2 byte value for game size in MiB in USBExtreme header
-skip non-valid entries in ul.cfg (useful for static size ul.cfg with empty records)

Pull Request checklist

Note: these are not necessarily requirements

  • I reformatted the code with clang-format
  • I checked to make sure my submission worked
  • I am the author of submission or have permission from the original author
  • Requires update of the PS2SDK
  • Requires update of the gsKit
  • Others (please specify below)

Pull Request description

@AKuHAK
Copy link
Member Author

AKuHAK commented Aug 11, 2021

@HowlingWolfHWC you might be interested in this

Copy link
Member

@KrahJohlito KrahJohlito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: oh I see, startup format appears to be ul.SLUS_123.45 so that’s why it was and needs to be 15 https://github.com/ps2homebrew/Open-PS2-Loader/blob/master/src/supportbase.c#L622

@AKuHAK
Copy link
Member Author

AKuHAK commented Aug 12, 2021

Edit: oh I see, startup format appears to be ul.SLUS_123.45 so that’s why it was and needs to be 15 https://github.com/ps2homebrew/Open-PS2-Loader/blob/master/src/supportbase.c#L622

So everything is fine?

@KrahJohlito
Copy link
Member

KrahJohlito commented Aug 12, 2021

Lol sorry I know that was confusing.. it’s just https://github.com/ps2homebrew/Open-PS2-Loader/blob/master/src/supportbase.c#L630 here that needs to be updated with your new changes since magic and startup are separate

edit: can’t multiline quote on my phone and my kid keeps taking it away.. but I’m sure you see what I mean, the whole function.. ul. Should be copied to magic and startup doesn’t need start copying to startup[3].. manual null after strncpy etc etc

@AKuHAK
Copy link
Member Author

AKuHAK commented Aug 12, 2021

Lol sorry I know that was confusing.. it’s just https://github.com/ps2homebrew/Open-PS2-Loader/blob/master/src/supportbase.c#L630 here that needs to be updated with your new changes since magic and startup are separate

edit: can’t multiline quote on my phone and my kid keeps taking it away.. but I’m sure you see what I mean, the whole function.. ul. Should be copied to magic and startup doesn’t need start copying to startup[3].. manual null after strncpy etc etc

oh, thanks, totally missed that function.

@AKuHAK AKuHAK marked this pull request as draft August 12, 2021 13:26
@AKuHAK
Copy link
Member Author

AKuHAK commented Aug 12, 2021

Probably I will remove that extended size handling as it seems that no one will use it, and implementation is a bit difficult for me :)

@AKuHAK AKuHAK closed this Aug 12, 2021
@AKuHAK AKuHAK reopened this Aug 12, 2021
@AKuHAK AKuHAK marked this pull request as ready for review August 12, 2021 19:43

for (i = 0; i < gamecount; i++) {
game = &(*list)[i];

if (game->format == GAME_FORMAT_USBLD && (i != excludeID)) {
memset(&GameEntry.startup[3], 0, GAME_STARTUP_MAX);
memcpy(GameEntry.startup, game->startup, GAME_STARTUP_MAX);
GameEntry.startup[GAME_STARTUP_MAX] = '\0';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still got an overflow mate, array gotta be -1

src/supportbase.c Outdated Show resolved Hide resolved
Copy link
Member Author

@AKuHAK AKuHAK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I learned tech a bit (google is my friend) and until we treat char sequence as a string everything is fine - we can do any operations (like fwrite, read, memcpy) without terminating NULL. But once we use the string function then it depends on the compiler, but in most cases, it will fill the string with the ending NULL character. So basically in our case GameEntry.name and GameEntry.startup is a character array, while game->name and game->startup can be both: and a character array, and a string. So what I finally mean, we should not call any string-based function, cause it will silently convert any char array into a string with terminating NULL. But using memcpy and fread, fwrite is valid usage and C compliant.

src/supportbase.c Outdated Show resolved Hide resolved
@KrahJohlito
Copy link
Member

Sounds good to me, even if the app is updated to allow space for null it would break compatibly with older versions.. Good to go yea?

@AKuHAK AKuHAK marked this pull request as draft August 13, 2021 07:20
Copy link
Member Author

@AKuHAK AKuHAK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me, even if the app is updated to allow space for null it would break compatibly with older versions.. Good to go yea?

Yeah I learned a lot about C today :D

src/supportbase.c Outdated Show resolved Hide resolved
@KrahJohlito
Copy link
Member

I’ve gone ahead and reverted the change so if you wanna continue working on this you can take your time and latest won’t be bugged.

@AKuHAK AKuHAK marked this pull request as ready for review August 13, 2021 09:47
@AKuHAK AKuHAK marked this pull request as draft August 13, 2021 13:03
@AKuHAK AKuHAK marked this pull request as ready for review August 16, 2021 19:54
@AKuHAK
Copy link
Member Author

AKuHAK commented Aug 16, 2021

Ok I tested it on real hardware - now USBExtreme games will show their real size in properties. Nothing is broken (as I know), so probably if code is ok, this PR can be merged.

Copy link
Member

@KrahJohlito KrahJohlito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@KrahJohlito KrahJohlito merged commit c43f188 into ps2homebrew:master Aug 17, 2021
@AKuHAK AKuHAK deleted the USBextreme_ext branch August 21, 2021 15:32
AKuHAK pushed a commit that referenced this pull request Sep 30, 2021
citronalco pushed a commit to citronalco/OPL-Daily-Builds that referenced this pull request Sep 10, 2023
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.

None yet

2 participants