Skip to content

Coverity updates #11551

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 11, 2019
Merged

Coverity updates #11551

merged 4 commits into from
Oct 11, 2019

Conversation

@Tharazi97
Copy link
Contributor

@Tharazi97 Tharazi97 commented Sep 23, 2019

Description

Changes to eliminate Coverity warnings.
Added simple initialization.
Added checking for null before using memcpy().

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@jamesbeyond @maciejbocianski

Release Notes

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 23, 2019 •
edited
Loading

One suggestion : instead of Coverity changes esp8266-driver commit, although it's simple, we can do: esp8266-driver: fix xyz variable init in ctor, second paragraph in the commit msg would be Fixes coverity issue about not initialized member" or something along the lines.

Entire msg would be:

esp8266-driver: fix xyz variable init in ctor

`Fixes coverity issue about not initialized member"

@ciarmcom ciarmcom requested review from a team, jamesbeyond and maciejbocianski September 23, 2019 13:00
@ciarmcom
Copy link
Member

ciarmcom commented Sep 23, 2019

@Tharazi97, thank you for your changes.
@maciejbocianski @jamesbeyond @ARMmbed/mbed-os-pan @ARMmbed/mbed-os-core @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

evedon
evedon reviewed Sep 23, 2019
View reviewed changes
drivers/source/FlashIAP.cpp Outdated
}

FlashIAP::FlashIAP() : _page_buf(nullptr)
FlashIAP::FlashIAP() : _page_buf(nullptr), _flash{nullptr}
Copy link
Contributor

@evedon evedon Sep 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

There is another PR already fixing this, see https://github.com/ARMmbed/mbed-os/pull/11494/files

cc @hugueskamba

Copy link
Contributor Author

@Tharazi97 Tharazi97 Sep 23, 2019

Choose a reason for hiding this comment

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

Removed from PR

@Tharazi97
esp8266-driver: fix variables init in constructor.
24cb033 
Fixes Coverity issue about not initialized members.
@Tharazi97
NFC-URI: fix passing null pointer to a function.
081183c 
Fixes Coverity issue about passing nullptr to memcpy.
@Tharazi97
NFC-Text: fix passing null pointer to a function.
d83cbb8 
Fixes Coverity issue about passing nullptr to memcpy.
@Tharazi97
NFCEEPROM: fix _ndef_buffer_reader init in ctor.
62d0770 
Fixes Coverity issue about not initialized members.
@0xc0170
Copy link
Contributor

0xc0170 commented Sep 24, 2019

CI started meanwhile

@mbed-ci
Copy link

mbed-ci commented Sep 24, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 25, 2019

Please review, this could go in if approved

hugueskamba
hugueskamba approved these changes Sep 25, 2019
View reviewed changes
Copy link
Collaborator

@hugueskamba hugueskamba left a comment

Choose a reason for hiding this comment

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

LGTM

features/nfc/source/nfc/ndef/common/Text.cpp
_text_record_size(other._text_record_size)
{
memcpy(_text_record, other._text_record, _text_record_size);
if (_text_record) {
Copy link
Collaborator

@hugueskamba hugueskamba Sep 25, 2019

Choose a reason for hiding this comment

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

Suggested change
if (_text_record) {
if (_text_record != nullptr) {

features/nfc/source/nfc/ndef/common/URI.cpp
{
_uri[uri_id_index] = id;
memcpy(_uri + uri_field_index, uri_field.data(), uri_field.size());
if (_uri) {
Copy link
Collaborator

@hugueskamba hugueskamba Sep 25, 2019

Choose a reason for hiding this comment

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

Suggested change
if (_uri) {
if (_uri != nullptr) {

features/nfc/source/nfc/ndef/common/URI.cpp
_uri_size(other._uri_size)
{
memcpy(_uri, other._uri, other._uri_size);
if (_uri) {
Copy link
Collaborator

@hugueskamba hugueskamba Sep 25, 2019

Choose a reason for hiding this comment

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

Suggested change
if (_uri) {
if (_uri != nullptr) {

Copy link
Contributor Author

@Tharazi97 Tharazi97 Sep 25, 2019

Choose a reason for hiding this comment

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

I wouldn't use that to keep consistency with other if statements.

Copy link
Collaborator

@hugueskamba hugueskamba Sep 25, 2019

Choose a reason for hiding this comment

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

No problem, already approved.

michalpasztamobica
michalpasztamobica approved these changes Sep 25, 2019
View reviewed changes
Copy link
Contributor

@michalpasztamobica michalpasztamobica left a comment

Choose a reason for hiding this comment

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

OK for the ESP8266Interface part.

@adbridge
Copy link
Contributor

adbridge commented Oct 7, 2019

Because this is 12 days old, going to rerun CI

@mbed-ci
Copy link

mbed-ci commented Oct 7, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

@jamesbeyond
Copy link
Contributor

jamesbeyond commented Oct 10, 2019

I think this is good to be merged ? @adbridge

@adbridge adbridge merged commit 64df072 into ARMmbed:master Oct 11, 2019
@0xc0170 0xc0170 mentioned this pull request Oct 16, 2019
Closed
@adbridge adbridge added release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 and removed release-version: 5.14.1 labels Oct 16, 2019
@0xc0170 0xc0170 added release-version: 5.15.0-rc1 and removed release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 labels Nov 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evedon evedon evedon left review comments

@hugueskamba hugueskamba hugueskamba approved these changes

@0xc0170 0xc0170 0xc0170 approved these changes

@jamesbeyond jamesbeyond Awaiting requested review from jamesbeyond

@maciejbocianski maciejbocianski Awaiting requested review from maciejbocianski

+1 more reviewer

@michalpasztamobica michalpasztamobica michalpasztamobica approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release-version: 5.15.0-rc1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@Tharazi97 @0xc0170 @ciarmcom @mbed-ci @adbridge @jamesbeyond @evedon @hugueskamba @michalpasztamobica