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

pkg/tinycbor: fix of compiler warnings "uninitialized variable used" #12505

Merged
merged 2 commits into from
Oct 27, 2019

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR patches the tinycbor package. On ESP32 and new ESP8266 (PR #11108) platforms, the compilation of the package fails since a local variable is potentially used uninitialized:

error: 'valf' may be used uninitialized in this function [-Werror=maybe-uninitialized]

With the patch provided in this PR, the variable is initialized with a default value and the according boards are removed from the balcklist.

Testing procedure

Just try to compile tests/tinycbor for a ESP32 board:

make BOARD=esp32-wroom-32 -C tests/pkg_tinycbor

Issues/PRs references

Required for PR #11108

@gschorcht gschorcht requested a review from bergzand October 19, 2019 12:26
@gschorcht gschorcht added Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Oct 19, 2019
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.

Small change, just fixes a compiler warning at the cost of an additional instruction.

Can you also submit a PR to https://github.com/intel/tinycbor while you're at it, so we can drop this patch in the future.

On ESP32 and new ESP8266 platform, the compilation of the package fails since a local variable is potentially used uninitialized. Therefore, the variable is initialized with a default value.
@gschorcht gschorcht force-pushed the pkg/tinycbor/compilation_fix branch from 2cca1e5 to b6b1252 Compare October 27, 2019 09:03
@gschorcht
Copy link
Contributor Author

@benpicco Can we merge it? It is required by #11108. After rebasing there are no conflicts with the master anymore.

@benpicco
Copy link
Contributor

It's all green, feel free to press the button 😉
I'd still would like it if you'd propose the patch upstream so we can drop it from our pkg in a future version.

@gschorcht
Copy link
Contributor Author

l green, feel free to press the button

@benpicco It's a bad style to merge own PRs. This should always be done by a reviewer or at least by another maintainer.

@benpicco benpicco merged commit 1a4f07a into RIOT-OS:master Oct 27, 2019
@benpicco
Copy link
Contributor

It's a bad style to merge own PRs.

That feels like unnecessary friction. If you want to adhere to that, the maintainer would have to go through all PRs again after they already ACKed them, just to see if CI has finished running.

I see that it would be a bad idea to merge if there is still an ongoing discussion, but when you got an uncontested ACK you are free to merge IMHO.

@gschorcht gschorcht deleted the pkg/tinycbor/compilation_fix branch October 27, 2019 12:26
@gschorcht
Copy link
Contributor Author

Thanks.

@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants