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.

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

Coverity updates #11551

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

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
Copy link
Member

@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
@@ -46,7 +46,7 @@ static inline bool is_aligned(uint32_t number, uint32_t alignment)
}
}

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

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
@@ -41,7 +41,9 @@ Text::Text(const Text &other) :
_text_record(other._text_record ? new uint8_t[other._text_record_size] : NULL),
_text_record_size(other._text_record_size)
{
memcpy(_text_record, other._text_record, _text_record_size);
if (_text_record) {
Copy link
Collaborator

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
@@ -40,15 +40,19 @@ URI::URI(uri_identifier_code_t id, const Span<const uint8_t> &uri_field) :
_uri(uri_field.size() ? new uint8_t[uri_id_code_size + uri_field.size()] : NULL),
_uri_size(uri_id_code_size + uri_field.size())
{
_uri[uri_id_index] = id;
memcpy(_uri + uri_field_index, uri_field.data(), uri_field.size());
if (_uri) {
Copy link
Collaborator

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::URI(const URI &other) :
_uri(other._uri ? new uint8_t[other._uri_size] : NULL),
_uri_size(other._uri_size)
{
memcpy(_uri, other._uri, other._uri_size);
if (_uri) {
Copy link
Collaborator

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

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

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

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

@michalpasztamobica michalpasztamobica michalpasztamobica approved these changes

@jamesbeyond jamesbeyond Awaiting requested review from jamesbeyond

@maciejbocianski maciejbocianski Awaiting requested review from maciejbocianski

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