shill: fix Nl80211Frame::ToString(), which has been broken since CL:289228 |
||
Issue descriptionNl80211Frame::ToString() has been broken since CL:289228, which introduced a bug where |Nl80211Frame::reason_code_string_| and |Nl80211Frame::status_code_string_| became never initialized.
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/8374b8c4012136eed7ce1f21dfb7bb50c3865277 commit 8374b8c4012136eed7ce1f21dfb7bb50c3865277 Author: Ben Chan <benchan@chromium.org> Date: Thu Nov 15 10:17:05 2018 shill: fix issues in Nl80211Frame::ToString() This CL addresses the following issues with Nl80211Frame: - Nl80211Frame::ToString() is broken since CL:289228, which introduced a bug where |Nl80211Frame::reason_code_string_| and |Nl80211Frame::status_code_string_| became never initialized. - The code relies on Nl80211Frame::InitFromPacket() to initialize |Nl80211Frame::reason_code_string_| and |Nl80211Frame::status_code_string_|, which doesn't guarantee that they aren't accessed before Nl80211Frame::InitFromPacket() is invoked. - Nl80211Frame::StringFromReason() and Nl80211Frame::StringFromReason() are implementation details of Nl80211Frame. It's unnecessary to expose them through the class definition of Nl80211Frame. Instead, these helpers are better defined under the IEEE_80211 namespace. BUG= chromium:904021 TEST=Run unit tests. Change-Id: I952ac6d545f487c726878084a180d5a1c8cf5d69 Reviewed-on: https://chromium-review.googlesource.com/1330124 Commit-Ready: Ben Chan <benchan@chromium.org> Tested-by: Ben Chan <benchan@chromium.org> Reviewed-by: Brian Norris <briannorris@chromium.org> [add] https://crrev.com/8374b8c4012136eed7ce1f21dfb7bb50c3865277/shill/net/ieee80211.cc [modify] https://crrev.com/8374b8c4012136eed7ce1f21dfb7bb50c3865277/shill/net/nl80211_message.h [modify] https://crrev.com/8374b8c4012136eed7ce1f21dfb7bb50c3865277/shill/BUILD.gn [add] https://crrev.com/8374b8c4012136eed7ce1f21dfb7bb50c3865277/shill/net/nl80211_message_test.cc [modify] https://crrev.com/8374b8c4012136eed7ce1f21dfb7bb50c3865277/shill/net/ieee80211.h [modify] https://crrev.com/8374b8c4012136eed7ce1f21dfb7bb50c3865277/shill/net/nl80211_message.cc
,
Nov 15
My fuzzer was going to catch this one :D
,
Dec 8
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/8eb370f60f962801a98efce8dd3b3ec84048ebd0 commit 8eb370f60f962801a98efce8dd3b3ec84048ebd0 Author: Brian Norris <briannorris@chromium.org> Date: Sat Dec 08 16:44:03 2018 common-mk: build with -Wunreachable-code This warning is not included in any of the default -Wall / -Wextra warning lists for clang at the moment (and it does nothing on gcc [1]), but it would have caught chromium:904021 a long time ago. Let's enable it by default. [1] https://gcc.gnu.org/ml/gcc-help/2011-05/msg00360.html CQ-DEPEND=CL:1351683, CL:1351840 BUG= chromium:904021 TEST=build shill, with and without unreachable code; precq Change-Id: Ibf715a2e9cff896c1e87a1f46efa1b747065312e Signed-off-by: Brian Norris <briannorris@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1330972 Reviewed-by: Manoj Gupta <manojgupta@chromium.org> Reviewed-by: Ben Chan <benchan@chromium.org> [modify] https://crrev.com/8eb370f60f962801a98efce8dd3b3ec84048ebd0/common-mk/common.mk [modify] https://crrev.com/8eb370f60f962801a98efce8dd3b3ec84048ebd0/common-mk/BUILD.gn [modify] https://crrev.com/8eb370f60f962801a98efce8dd3b3ec84048ebd0/common-mk/common.gypi |
||
►
Sign in to add a comment |
||
Comment 1 by benchan@google.com
, Nov 10