From acecaf721664c9aa1c47f06c3a60501428b1c9b6 Mon Sep 17 00:00:00 2001
From: Mark Salyzyn <salyzyn@google.com>
Date: Fri, 10 Aug 2018 08:15:57 -0700
Subject: [PATCH] llkd: llkSplit should prevent empty entries

Add "false" as an option fed into llkSplit to be equivalent to empty,
as a truly empty list is replaced with the internal defaults.  Ensure
that no empty entries are added to the returned list.  Add some
additional provisos to README.md, as well as the explanation of what
"false" means for the associated properties.

Test: llkd_unit_test
Bug: 33808187
Bug: 111910505
Bug: 80502612
Change-Id: Iac0457ea1f6cd559b0875f9871dbae839001276d
---
 llkd/README.md   | 26 ++++++++++++++++++++------
 llkd/libllkd.cpp | 18 +++++++++++-------
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/llkd/README.md b/llkd/README.md
index b2ba2a2f6..231458365 100644
--- a/llkd/README.md
+++ b/llkd/README.md
@@ -60,7 +60,7 @@ kernel instead of deal with more graceful kill operation.
 Android Properties
 ------------------
 
-Android Properties llkd respond to (<prop>_ms parms are in milliseconds):
+Android Properties llkd respond to (*prop*_ms parms are in milliseconds):
 
 #### ro.config.low_ram
 default false, if true do not sysrq t (dump all threads).
@@ -99,19 +99,33 @@ default 2 minutes samples of threads for D or Z.
 #### ro.llk.blacklist.process
 default 0,1,2 (kernel, init and [kthreadd]) plus process names
 init,[kthreadd],[khungtaskd],lmkd,lmkd.llkd,llkd,watchdogd,
-[watchdogd],[watchdogd/0],...,[watchdogd/<get_nprocs-1>].
+[watchdogd],[watchdogd/0],...,[watchdogd/***get_nprocs**-1*].
+The string false is the equivalent to an empty list.
+Do not watch these processes.  A process can be comm, cmdline or pid reference.
+NB: automated default here can be larger than the current maximum property
+size of 92.
+NB: false is a very very very unlikely process to want to blacklist.
 
 #### ro.llk.blacklist.parent
 default 0,2 (kernel and [kthreadd]).
+The string false is the equivalent to an empty list.
+Do not watch processes that have this parent.
+A parent process can be comm, cmdline or pid reference.
 
 #### ro.llk.blacklist.uid
-default <empty>, comma separated list of uid numbers or names.
+default *empty* or false, comma separated list of uid numbers or names.
+The string false is the equivalent to an empty list.
+Do not watch processes that match this uid.
 
 Architectural Concerns
 ----------------------
 
+- built-in [khungtask] daemon is too generic and trips on driver code that
+  sits around in D state too much.  To switch to S instead makes the task(s)
+  killable, so the drivers should be able to resurrect them if needed.
+- Properties are limited to 92 characters.
 - Create kernel module and associated gTest to actually test panic.
-- Create gTest to test out blacklist (ro.llk.blacklist.<properties> generally
+- Create gTest to test out blacklist (ro.llk.blacklist.*properties* generally
   not be inputs).  Could require more test-only interfaces to libllkd.
-- Speed up gTest using something else than ro.llk.<properties>, which should
-  not be inputs.
+- Speed up gTest using something else than ro.llk.*properties*, which should
+  not be inputs as they should be baked into the product.
diff --git a/llkd/libllkd.cpp b/llkd/libllkd.cpp
index 3b2877560..48551f264 100644
--- a/llkd/libllkd.cpp
+++ b/llkd/libllkd.cpp
@@ -285,7 +285,7 @@ struct proc {
           schedUpdate(0),
           nrSwitches(0),
           update(llkUpdate),
-          count(0),
+          count(0ms),
           pid(pid),
           ppid(ppid),
           uid(-1),
@@ -574,15 +574,19 @@ std::string llkFormat(const std::unordered_set<std::string>& blacklist) {
 
 // We only officially support comma separators, but wetware being what they
 // are will take some liberty and I do not believe they should be punished.
-std::unordered_set<std::string> llkSplit(const std::string& s,
-                                         const std::string& delimiters = ", \t:") {
+std::unordered_set<std::string> llkSplit(const std::string& s) {
     std::unordered_set<std::string> result;
 
+    // Special case, allow boolean false to empty the list, otherwise expected
+    // source of input from android::base::GetProperty will supply the default
+    // value on empty content in the property.
+    if (s == "false") return result;
+
     size_t base = 0;
-    size_t found;
-    while (true) {
-        found = s.find_first_of(delimiters, base);
-        result.emplace(s.substr(base, found - base));
+    while (s.size() > base) {
+        auto found = s.find_first_of(", \t:", base);
+        // Only emplace content, empty entries are not an option
+        if (found != base) result.emplace(s.substr(base, found - base));
         if (found == s.npos) break;
         base = found + 1;
     }