Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[develop] Add datel cheat support from filesystem #204
base: develop
Are you sure you want to change the base?
[develop] Add datel cheat support from filesystem #204
Changes from 25 commits
db6a7b8
bfc28b4
cd26c23
2b52a92
e9cb8d1
9ad2ec1
d308f67
fabe4b1
9123c8e
1afa136
c39142d
466684c
0c61055
85ad8d0
df26ee0
5a73092
1bd32a3
bde4a79
fc23d60
7f6630f
10c1b0c
14a6af4
a1e9cc5
d9c0a38
8181626
6f25047
8165d8c
8634424
f293848
b8f3bcc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Fix memory leak when
fread
failsIf
fread
fails,cheatsContent
is not freed before returning, leading to a memory leak.Apply this diff to fix the issue:
if(fread(cheatsContent, cheatsLength, 1, cheatsFile) != 1) { path_free(path); + free(cheatsContent); return CHEAT_LOAD_ERR_READ_FAILED; }
📝 Committable suggestion
🧰 Tools
🪛 cppcheck (2.10-2)
[error] 152-152: Memory leak
(memleak)
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.
Fix memory leak when
fclose
failsIf
fclose
fails,cheatsContent
is not freed before returning, leading to a memory leak.Apply this diff to fix the issue:
if(fclose(cheatsFile) != 0){ path_free(path); + free(cheatsContent); return CHEAT_LOAD_ERR_CLOSE_FAILED; }
📝 Committable suggestion
🧰 Tools
🪛 cppcheck (2.10-2)
[error] 158-158: Memory leak
(memleak)
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.
Correct the initialization of
lines
Initializing
lines
to1
may lead to an incorrect count of cheat entries.Initialize
lines
to0
to accurately count the number of lines: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.
Adjust the allocation size for
cheats
arrayThe calculation for the size of the
cheats
array may lead to insufficient memory allocation.Update the allocation size to accommodate all cheat entries and the terminating zeros:
📝 Committable suggestion
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.
something funky going on here... need to check
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.
Current thinking is that this block
is in the wrong place and should be seperate from the ROM info file.
Though I have not looked into where it should actually be.
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.
So i wrote this before I had this next idea:
Ideally, the .ini file stores an array or something like that) of cheat titles/indexes (whatever ends up working best), so that rather than having an on-off cheat, the menu will parse previously enabled cheats.
This approach would ensure errors would ahve been shown to the user at some point, and thus only working/enabled cheats are included
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.
Fix the string comparison logic.
The
strncmp
condition is incorrect. It returns 0 for equal strings, so the current logic is inverted.Apply this diff to fix the logic:
📝 Committable suggestion
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.
Are we happy with this extension name?!
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.
This allows users to kinda drag-and-drop files from other sources(PJ64 for ex) with only some reformatting. Totally open to other extensions though