Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

drivers/nrf24l01p: fix potentially undefined return value #12587

Merged
merged 1 commit into from
Oct 28, 2019

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Oct 27, 2019

Contribution description

This PR fixes a potentially uninitialized returned value in the nrf24l01p driver.
The problem is raised when running scan-build with the LLVM toolchain.

After checking the datasheet, it seems that the returned value is quite deterministic (4 possible values, coded on 2 bits). So using using a map containing the expected power values should work.

Testing procedure

  • Verify scan-build is fixed:
    $ TOOLCHAIN=llvm make -C tests/driver_nrf24l01p_lowlevel scan-build
    
  • Verify the driver works (I couldn't test because of no hardware available)

Issues/PRs references

Tick one item in #11852

@aabadie aabadie added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 27, 2019
@aabadie aabadie force-pushed the pr/drivers/nrf24l01p_scan_build branch from 9869557 to 283a906 Compare October 27, 2019 17:29
@@ -618,30 +618,13 @@ int nrf24l01p_set_power(const nrf24l01p_t *dev, int pwr)
return nrf24l01p_write_reg(dev, REG_RF_SETUP, rf_setup);
}

static const int _nrf24l01p_power_map[4] = { -18, -12, -6, 0 };
Copy link
Contributor

Choose a reason for hiding this comment

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

You can save a few bytes and use int8_t[] here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested and this makes no difference in code size. Anyway I think it's better to be explicit and use int8_t instead of int.
I pushed the change, directly squashed.

@aabadie aabadie force-pushed the pr/drivers/nrf24l01p_scan_build branch from 283a906 to 808e442 Compare October 28, 2019 13:47
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Code looks much cleaner now, no functional change.

@aabadie aabadie merged commit e075d97 into RIOT-OS:master Oct 28, 2019
@aabadie aabadie deleted the pr/drivers/nrf24l01p_scan_build branch October 28, 2019 14:48
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants