Skip to content

Fix for a2m -Wsign-compare#144

Merged
mywave82 merged 2 commits into
adplug:masterfrom
mywave82:a2m-Wsign-compare
Feb 26, 2023
Merged

Fix for a2m -Wsign-compare#144
mywave82 merged 2 commits into
adplug:masterfrom
mywave82:a2m-Wsign-compare

Conversation

@mywave82

Copy link
Copy Markdown
Contributor

src/a2m.cpp: In member function ‘virtual bool Ca2mLoader::load(const string&, const CFileProvider&)’: src/a2m.cpp:162:17: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
162 | for (i = 0; i < length; i++)
src/a2m.cpp: In member function ‘virtual bool Ca2mLoader::load(const string&, const CFileProvider&)’:
src/a2m.cpp:203:20: warning: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
203 | if (orgptr - org < needed) {

src/a2m.cpp: In member function ‘virtual bool Ca2mLoader::load(const string&, const CFileProvider&)’:
src/a2m.cpp:162:17: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
  162 |   for (i = 0; i < length; i++)
src/a2m.cpp: In member function ‘virtual bool Ca2mLoader::load(const string&, const CFileProvider&)’:
src/a2m.cpp:203:20: warning: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
  203 |   if (orgptr - org < needed) {
@mywave82

Copy link
Copy Markdown
Contributor Author

There are more of these signed compare warnings, but probably makes sense to split them up into PRs, since some of them might need discussion? Or do you prefer to have them in a single PR?

Comment thread src/a2m.cpp Outdated
@binarymaster

Copy link
Copy Markdown
Member

There are more of these signed compare warnings, but probably makes sense to split them up into PRs, since some of them might need discussion?

Separate PRs for each would probably be better, because easier to review and merge.

@mywave82 mywave82 merged commit 4698c88 into adplug:master Feb 26, 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.

2 participants