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

Various warning fixes #663

Merged
merged 4 commits into from
Aug 26, 2024
Merged

Various warning fixes #663

merged 4 commits into from
Aug 26, 2024

Conversation

Archez
Copy link
Contributor

@Archez Archez commented Aug 25, 2024

Various warning fixes including:

  • warning: format not a string literal and no format arguments [-Wformat-security]
  • warning: ‘typedef’ was ignored in this declaration
  • warning: ‘XYZ’ will be initialized after [-Wreorder]
  • a couple integer size/sign values

Comment on lines 139 to 145
if (dots.size() > 1) {
int curDot = 0;
for (auto& dot : dots) {
if (gjson2.contains(dot)) {
if (curDot == dots.size()) {
if (dots.size() == 0) {
gjson2.erase(dot);
} else {
gjson2 = gjson2[dot];
Copy link
Contributor Author

@Archez Archez Aug 25, 2024

Choose a reason for hiding this comment

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

@Malkierian you added this method, so I just wanted to confirm with you that curDot was only ever meant to be 0 and that this change is fine. Otherwise did you mean for curDot to be re-assigned in this for-loop for some logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on discord discussion, seems like this method is possibly incomplete and will be looked into in a follow up PR.

For now, I've reverted the change and only applied the signed int comparison fix to satisfy the warning.

@Kenix3 Kenix3 merged commit b59bf87 into Kenix3:main Aug 26, 2024
5 checks passed
@Archez Archez deleted the warning-fixes-1 branch August 26, 2024 17:21
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.

3 participants