From 956fd40b6145c4aba9160e10bfc1ea609873ce8d Mon Sep 17 00:00:00 2001
From: mukesh agrawal <quiche@google.com>
Date: Wed, 20 Apr 2016 13:52:27 -0700
Subject: WifiLogger: fix bug in HAL callback registration

When we call WifiLogger.startLogging(), WifiLogger
may or may not register callbacks for ring-buffer
data delivery, and WiFi alerts.

Whether or not WifiLogger registers the callbacks
depends on whether or not the callbacks have already
been reigstered. If WifiLogger has already registered
callbacks, then a new call to startLogging() will
skip callback registration.

Now, it's not entirely clear if this already-registered
check is necessary. It is certainly conceivable that the
HAL implementations allow us to replace existing callbacks,
without explicitly removing the existing callbacks first.

But, the necessity of the check aside, the immediate
problem is that the already-registered check fails to
handle the case where an early registration failed.

This CL revises the code, to handle this case.

While there:
- Fix some whitespace issues.
- Remove unnecessary comment about the effects of
  resetLogHandler(). (Even if the comment _were_ necessary,
  it should be part of the Javadoc for resetLogHandler().)

BUG=28274991
TEST=(new) unit tests, manual

Manual test:
- boot device
$ adb shell dumpsys | grep -A1 'ring-buffer = driver_prints_rb'
ring-buffer = driver_prints_rb
<base64 encoded data...>

Change-Id: I04e9bc1863aaf5bda00ca9cf8dc35696ae1e5f24
---
 service/java/com/android/server/wifi/WifiLogger.java | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

(limited to 'service')

diff --git a/service/java/com/android/server/wifi/WifiLogger.java b/service/java/com/android/server/wifi/WifiLogger.java
index 6d7b608a8..3ffe5837b 100644
--- a/service/java/com/android/server/wifi/WifiLogger.java
+++ b/service/java/com/android/server/wifi/WifiLogger.java
@@ -96,6 +96,7 @@ class WifiLogger extends BaseWifiLogger {
             "Driver state dump";
 
     private int mLogLevel = VERBOSE_NO_LOG;
+    private boolean mIsLoggingEventHandlerRegistered;
     private WifiNative.RingBufferStatus[] mRingBuffers;
     private WifiNative.RingBufferStatus mPerPacketRingBuffer;
     private WifiStateMachine mWifiStateMachine;
@@ -106,6 +107,7 @@ class WifiLogger extends BaseWifiLogger {
             WifiStateMachine wifiStateMachine, WifiNative wifiNative) {
         mWifiStateMachine = wifiStateMachine;
         mWifiNative = wifiNative;
+        mIsLoggingEventHandlerRegistered = false;
     }
 
     @Override
@@ -114,8 +116,9 @@ class WifiLogger extends BaseWifiLogger {
         mDriverVersion = mWifiNative.getDriverVersion();
         mSupportedFeatureSet = mWifiNative.getSupportedLoggerFeatureSet();
 
-        if (mLogLevel == VERBOSE_NO_LOG)
-            mWifiNative.setLoggingEventHandler(mHandler);
+        if (!mIsLoggingEventHandlerRegistered) {
+            mIsLoggingEventHandlerRegistered = mWifiNative.setLoggingEventHandler(mHandler);
+        }
 
         if (verboseEnabled) {
             mLogLevel = VERBOSE_LOG_WITH_WAKEUP;
@@ -162,14 +165,17 @@ class WifiLogger extends BaseWifiLogger {
 
     @Override
     public synchronized void stopLogging() {
-        if (mLogLevel != VERBOSE_NO_LOG) {
-            //resetLogHandler only can be used when you terminate all logging since all handler will
-            //be removed. This also stop alert logging
-            if(!mWifiNative.resetLogHandler()) {
+        if (mIsLoggingEventHandlerRegistered) {
+            if (!mWifiNative.resetLogHandler()) {
                 Log.e(TAG, "Fail to reset log handler");
             } else {
-                if (DBG) Log.d(TAG,"Reset log handler");
+                if (DBG) Log.d(TAG, "Reset log handler");
             }
+            // Clear mIsLoggingEventHandlerRegistered even if resetLogHandler() failed, because
+            // the log handler is in an indeterminate state.
+            mIsLoggingEventHandlerRegistered = false;
+        }
+        if (mLogLevel != VERBOSE_NO_LOG) {
             stopLoggingAllBuffers();
             mRingBuffers = null;
             mLogLevel = VERBOSE_NO_LOG;
-- 
cgit v1.2.3