Skip to content

Conversation

@iltis42
Copy link
Contributor

@iltis42 iltis42 commented Jan 2, 2022

Summary

Remove useless parameter check:

BTScanResultsSet.cpp:67:8: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
if (i < 0)
^
[ 83%] Linking C static library libarduino-esp32.a

Impact

Please describe impact of your PR and it's function.

Related links

Please provide links to related issue, PRs etc.

Remove useless parameter check:

BTScanResultsSet.cpp:67:8: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
  if (i < 0)
      ~~^~~
[ 83%] Linking C static library libarduino-esp32.a
@CLAassistant
Copy link

CLAassistant commented Jan 2, 2022

CLA assistant check
All committers have signed the CLA.

@iltis42 iltis42 changed the title Update BTScanResultsSet.cpp Remove useless parameter check for unit < 0 in BTScanResultsSet.cpp Jan 2, 2022
@VojtechBartoska
Copy link
Contributor

@iltis42 Thanks for your PR. Description edited, please consider filling out PR description more specifically next time. It help us to understand more to your changes.

@mrengineer7777
Copy link
Collaborator

This commit changes functionality: BTScanResultsSet::getDevice() will not be able to check for negative values. Consider:
BTScanResultsSet::getCount() returns int.
BTScanResultsSet::dump() passes int to getDevice().

I believe the correct fix here is to change the function signature to getDevice(int32_t i). It should also be updated in BTScan.h.

@iltis42
Copy link
Contributor Author

iltis42 commented Jan 7, 2022

This commit changes functionality: BTScanResultsSet::getDevice() will not be able to check for negative values. Consider: BTScanResultsSet::getCount() returns int. BTScanResultsSet::dump() passes int to getDevice().

I believe the correct fix here is to change the function signature to getDevice(int32_t i). It should also be updated in BTScan.h.

In an isolated view of this change, there is no change of functionality. But fully agree here. Changing the signature will make this change work. Bingo!

@iltis42
Copy link
Contributor Author

iltis42 commented Jan 7, 2022

Closed

@iltis42
Copy link
Contributor Author

iltis42 commented Jan 7, 2022

New PR is here:
#6109

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