New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 615580 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Code cleanup: Remove ("if !defined(DISABLED_WAKE_ON_WIFI)) from shill's wifi codebase

Project Member Reported by kirtika@google.com, May 27 2016

Issue description

<b>Version: <Kenneth, what is the frequency?></b>
<b>OS: <please tell me it's not XP></b>

What steps will reproduce the problem?
(1)
(2)
(3)

What is the expected output?

What do you see instead?


Please use labels and text to provide additional information.

 

Comment 1 by kirtika@google.com, May 28 2016

Cc: semenzato@chromium.org jleong@chromium.org kirtika@chromium.org snanda@chromium.org tbroch@chromium.org
Labels: -Pri-3 Code-Improvement shill Pri-1
Summary: Code cleanup: Remove ("if !defined(DISABLED_WAKE_ON_WIFI)) from shill's wifi codebase (was: Code cleanup: Remove ("if !defined(DISABLED_WAKe_)
The following files have "#if !defined(DISABLED_WAKE_ON_WIFI), #else, #endif" blocks scattered in almost all functions. 
- wifi/wake_on_wifi.cc 
- wifi/wake_on_wifi.h
- wifi/wake_on_wifi_unittest.cc

The last file has a ~3000 line if-else-endif block. 
This is greatly increasing the risk that any changes/new developments will lead to bugs because of both cases not being correctly handled. Such bugs have been noticed during development of new metrics for wifi. 

Suggest coming up with an implementation that does not link in wifi/wake_on_wifi.cc file if wake-on-wifi is disabled. 
Suggest removing double negatives (if !defined(DISABLED_WAKE_ON_WIFI)) --> (if ENABLED_WAKE_ON_wIFI)) 

Comment 2 by kirtika@google.com, May 28 2016

Components: OS>Systems>Network
Project Member

Comment 3 by sheriffbot@chromium.org, Jun 2 2016

Labels: Hotlist-Google
Cc: cernekee@chromium.org

Sign in to add a comment