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

[build] fix warning of clearing non-trivial type #5710

Merged
merged 3 commits into from Oct 27, 2020
Merged

[build] fix warning of clearing non-trivial type #5710

merged 3 commits into from Oct 27, 2020

Conversation

wgtdkp
Copy link
Member

@wgtdkp wgtdkp commented Oct 26, 2020

There is a warning with gcc/g++ 10.2.0 when clearing an object of non-trivial type with memset:

(build command: cmake -GNinja -DOT_COMMISSIONER=ON .. && ninja)

OPENTHREAD_CONFIG_THREAD_VERSION=OT_THREAD_VERSION_1_1
[65/132] Building CXX object src/core/CMakeFiles/openthread-ftd.dir/meshcop/commissioner.cpp.o
../src/core/meshcop/commissioner.cpp: In constructor ‘ot::MeshCoP::Commissioner::Commissioner(ot::Instance&)’:
../src/core/meshcop/commissioner.cpp:78:41: warning: ‘void* memset(void*, int, size_t)’ clearing an object of non-trivial type ‘struct ot::MeshCoP::Commissioner::Joiner’; use assignment or value-initialization instead [-Wclass-memaccess]
   78 |     memset(mJoiners, 0, sizeof(mJoiners));
      |                                         ^
In file included from ../src/core/meshcop/commissioner.cpp:34:
../src/core/meshcop/commissioner.hpp:347:12: note: ‘struct ot::MeshCoP::Commissioner::Joiner’ declared here
  347 |     struct Joiner
      |            ^~~~~~
[132/132] Linking CXX static library src/ncp/libopenthread-ncp-ftd.a

This PR fixes this issue:

  1. remove the problematic memset usages;
  2. turn on OT_COMPILE_WARNING_AS_ERROR for all CI builds;

@google-cla google-cla bot added the cla: yes label Oct 26, 2020
@codecov
Copy link

codecov bot commented Oct 26, 2020 •
edited

Codecov Report

Merging #5710 into master will increase coverage by 4.59%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #5710      +/-   ##
==========================================
+ Coverage   78.07%   82.67%   +4.59%     
==========================================
  Files         378      378              
  Lines       48145    48447     +302     
==========================================
+ Hits        37589    40052    +2463     
+ Misses      10556     8395    -2161
Impacted Files Coverage Δ
src/core/meshcop/commissioner.cpp 84.80% <100.00%> (+3.33%) ⬆️
src/core/thread/network_data_leader_ftd.cpp 86.19% <100.00%> (+3.70%) ⬆️
examples/apps/ncp/main.c 46.42% <0.00%> (-3.58%) ⬇️
src/core/thread/dua_manager.hpp 60.00% <0.00%> (-2.50%) ⬇️
src/core/thread/lowpan.hpp 97.61% <0.00%> (-2.39%) ⬇️
src/core/thread/topology.hpp 98.18% <0.00%> (-1.82%) ⬇️
src/core/utils/jam_detector.cpp 17.64% <0.00%> (-0.94%) ⬇️
src/lib/spinel/spinel_buffer.cpp 97.84% <0.00%> (-0.81%) ⬇️
src/core/net/dhcp6_client.cpp 94.96% <0.00%> (-0.78%) ⬇️
src/core/backbone_router/bbr_local.cpp 93.82% <0.00%> (-0.21%) ⬇️
... and 130 more

@size-report
Copy link

size-report bot commented Oct 26, 2020 •
edited

Size Report of OpenThread

Merging #5710 into master(12b967c).

name branch text data bss total
ot-cli-ftd_1.1 master 384092 736 72700 457528
#5710 384092 736 72700 457528
+/- 0 0 0 0
ot-cli-mtd_1.1 master 311580 736 64116 376432
#5710 311580 736 64116 376432
+/- 0 0 0 0
ot-ncp-ftd_1.1 master 378028 736 71980 450744
#5710 378028 736 71980 450744
+/- 0 0 0 0
ot-ncp-mtd_1.1 master 308508 736 63396 372640
#5710 308508 736 63396 372640
+/- 0 0 0 0
ot-rcp_1.1 master 64296 636 7512 72444
#5710 64296 636 7512 72444
+/- 0 0 0 0
libopenthread-cli-ftd.a_1.1 master 38248 0 3108 41356
#5710 38248 0 3108 41356
+/- 0 0 0 0
libopenthread-cli-mtd.a_1.1 master 31067 0 3108 34175
#5710 31067 0 3108 34175
+/- 0 0 0 0
libopenthread-ftd.a_1.1 master 189887 4 54992 244883
#5710 189887 4 54992 244883
+/- 0 0 0 0
libopenthread-mtd.a_1.1 master 129907 4 46408 176319
#5710 129907 4 46408 176319
+/- 0 0 0 0
libopenthread-ncp-ftd.a_1.1 master 48147 0 2388 50535
#5710 48147 0 2388 50535
+/- 0 0 0 0
libopenthread-ncp-mtd.a_1.1 master 43343 0 2388 45731
#5710 43343 0 2388 45731
+/- 0 0 0 0
libopenthread-rcp.a_1.1 master 27198 0 1532 28730
#5710 27198 0 1532 28730
+/- 0 0 0 0
libopenthread-radio.a_1.1 master 13386 0 150 13536
#5710 13386 0 150 13536
+/- 0 0 0 0
ot-cli-ftd_1.2 master 404964 736 81660 487360
#5710 404964 736 81660 487360
+/- 0 0 0 0
ot-cli-mtd_1.2 master 320292 736 64332 385360
#5710 320292 736 64332 385360
+/- 0 0 0 0
ot-ncp-ftd_1.2 master 396708 736 80940 478384
#5710 396708 736 80940 478384
+/- 0 0 0 0
ot-ncp-mtd_1.2 master 316388 736 63612 380736
#5710 316388 736 63612 380736
+/- 0 0 0 0
ot-rcp_1.2 master 69280 636 16316 86232
#5710 69280 636 16316 86232
+/- 0 0 0 0
libopenthread-cli-ftd.a_1.2 master 39576 0 3108 42684
#5710 39576 0 3108 42684
+/- 0 0 0 0
libopenthread-cli-mtd.a_1.2 master 31511 0 3108 34619
#5710 31511 0 3108 34619
+/- 0 0 0 0
libopenthread-ftd.a_1.2 master 209273 4 63896 273173
#5710 209273 4 63896 273173
+/- 0 0 0 0
libopenthread-mtd.a_1.2 master 137864 4 46568 184436
#5710 137864 4 46568 184436
+/- 0 0 0 0
libopenthread-ncp-ftd.a_1.2 master 48147 0 2388 50535
#5710 48147 0 2388 50535
+/- 0 0 0 0
libopenthread-ncp-mtd.a_1.2 master 43343 0 2388 45731
#5710 43343 0 2388 45731
+/- 0 0 0 0
libopenthread-rcp.a_1.2 master 27198 0 1532 28730
#5710 27198 0 1532 28730
+/- 0 0 0 0
libopenthread-radio.a_1.2 master 14082 0 150 14232
#5710 14082 0 150 14232
+/- 0 0 0 0

@wgtdkp wgtdkp requested a review from abtink October 26, 2020 07:05
@simonlingoogle
Copy link
Member

Note that code size increased by about 300B. @wgtdkp

@wgtdkp
Copy link
Member Author

wgtdkp commented Oct 26, 2020

Note that code size increased by about 300B. @wgtdkp

Yes, we introduced a new default constructor.

abtink
abtink reviewed Oct 26, 2020
View reviewed changes
Copy link
Member

@abtink abtink left a comment

Choose a reason for hiding this comment

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

I noticed this PR is increasing code size (ncp/cli ftd) by ~270 bytes (I guess from extra constructor calls).

Some suggestions below where I think we can fix/address the compiler warning and avoid the overhead.

src/core/meshcop/commissioner.cpp Show resolved Hide resolved
src/core/common/time.hpp Outdated Show resolved Hide resolved
src/core/thread/network_data_leader_ftd.cpp Show resolved Hide resolved
simonlingoogle
simonlingoogle approved these changes Oct 27, 2020
View reviewed changes
Copy link
Member

@simonlingoogle simonlingoogle left a comment

Choose a reason for hiding this comment

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

LGTM 👍

abtink
abtink approved these changes Oct 27, 2020
View reviewed changes
Copy link
Member

@abtink abtink left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks 👍

@wgtdkp wgtdkp requested a review from jwhui October 27, 2020 08:39
@jwhui jwhui merged commit 3de6b9a into openthread:master Oct 27, 2020
simonlingoogle pushed a commit to simonlingoogle/openthread that referenced this pull request Nov 12, 2020
[Github Sync] Merge github/master to master
992f3ab 
Bug: 171933690

* origin/github/master:
  [low-power] implement forward tracking series (openthread#5608)
  [efr32] radio: increase FIFO size to allow handling of heavy traffic (openthread#5742)
  [test] verify realm-local multicast on POSIX (openthread#5738)
  [clang-tidy] google-explicit-constructor (openthread#5734)
  [clang-tidy] misc-unused-using-decls (openthread#5732)
  [continuous-integration] use lcov to generate coverage data (openthread#5635)
  [type-traits] add 'IsPointer<Type>' and use it in message/dataset (openthread#5735)
  [posix] set default IPv6 hop limit to OPENTHREAD_CONFIG_IP6_HOP_LIMIT_DEFAULT (openthread#5736)
  [net-diag] invoke the callback when failed to get the response (openthread#5653)
  [site] fix border router guide typo in network (openthread#5731)
  [cli] update cli doc (openthread#5729)
  [thread-cert] refactor case 5.1.3 using pktverify (openthread#5708)
  [thread-cert] refactor case 5.1.2 using pktverify (openthread#5686)
  [test] arm build with cmake 3.10 (openthread#5727)
  [clang-tidy] google-readability-casting (openthread#5720)
  [clang-tidy] modernize-use-equals-default (openthread#5719)
  [clang-tidy] readability-avoid-const-params-in-decls (openthread#5717)
  [clang-tidy] modernize-use-equals-delete (openthread#5718)
  [efr32] add MG21/BRD4180B support (openthread#5725)
  [posix] fix cast-align build error on ARM (openthread#5672)
  [logging] add optional `otPlatLogLine()` & use it in NCP/CLI (openthread#5704)
  [clang-tidy] readability-simplify-boolean-expr (openthread#5716)
  [clang-tidy] modernize-use-nullptr (openthread#5715)
  [dataset] add new helper 'SetTlv()' with a template 'ValueType' (openthread#5722)
  [dataset-manager] simplify 'SendSetRequest()' (openthread#5721)
  [nrf528xx] fix SPI issue (openthread#5703)
  [clang-tidy] modernize-use-bool-literals (openthread#5714)
  [cmake] no target_link_options in cmake 3.10 (openthread#5706)
  [low-power] enhance CSL transmission on NRF52840 using `transmit_at` (openthread#5545)
  [build] fix warning of clearing non-trivial type (openthread#5710)
  [scripts] add MLR Backbone multicast routing test (openthread#5578)
  [mlr] fix build when OPENTHREAD_CONFIG_IP6_SLAAC_ENABLE is disabled (openthread#5711)
  [cmake] use init flags in toolchain files (openthread#5705)
  [cmake] set default build type to Debug (openthread#5709)
  [thread-cert] refactor case 5.1.1 using pktverify (openthread#5680)
  [posix] fix udp close (openthread#5690)
Change-Id: Ic7e4051040f093aa6dce9de5e2031a8accddec87
sjlongland pushed a commit to vrtsystems/openthread that referenced this pull request Dec 14, 2022
@wgtdkp @sjlongland
WSHUB-644: openthread [build] fix warning of clearing non-trivial type (
3ac39ee 
openthread#5710)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jwhui jwhui jwhui approved these changes

@abtink abtink abtink approved these changes

@librasungirl librasungirl librasungirl approved these changes

@simonlingoogle simonlingoogle simonlingoogle approved these changes

Assignees
No one assigned
Labels
cla: yes comp: build enhancement P4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@wgtdkp @simonlingoogle @jwhui @abtink @librasungirl