Merge "libgui: use read/writeStrongBinder" into stage-aosp-master
diff --git a/cmds/dumpstate/Android.bp b/cmds/dumpstate/Android.bp
index 4020480..16b55a5 100644
--- a/cmds/dumpstate/Android.bp
+++ b/cmds/dumpstate/Android.bp
@@ -146,7 +146,11 @@
     ],
     static_libs: ["libgmock"],
     test_config: "dumpstate_test.xml",
-    data: [":dumpstate_test_fixture", "tests/testdata/**/*"]
+    data: [
+        ":dumpstate_test_fixture",
+        "tests/testdata/**/*",
+    ],
+    test_suites: ["device-tests"],
 }
 
 cc_test {
diff --git a/cmds/dumpstate/TEST_MAPPING b/cmds/dumpstate/TEST_MAPPING
new file mode 100644
index 0000000..083944f
--- /dev/null
+++ b/cmds/dumpstate/TEST_MAPPING
@@ -0,0 +1,7 @@
+{
+  "presubmit": [
+    {
+      "name": "dumpstate_test"
+    }
+  ]
+}
\ No newline at end of file
diff --git a/cmds/service/service.cpp b/cmds/service/service.cpp
index d5dc6b7..543357c 100644
--- a/cmds/service/service.cpp
+++ b/cmds/service/service.cpp
@@ -70,7 +70,7 @@
 {
     bool wantsUsage = false;
     int result = 0;
-    
+
     while (1) {
         int ic = getopt(argc, argv, "h?");
         if (ic < 0)
@@ -97,7 +97,7 @@
         aerr << "service: Unable to get default service manager!" << endl;
         return 20;
     }
-    
+
     if (optind >= argc) {
         wantsUsage = true;
     } else if (!wantsUsage) {
@@ -119,8 +119,8 @@
             for (unsigned i = 0; i < services.size(); i++) {
                 String16 name = services[i];
                 sp<IBinder> service = sm->checkService(name);
-                aout << i 
-                     << "\t" << good_old_string(name) 
+                aout << i
+                     << "\t" << good_old_string(name)
                      << ": [" << good_old_string(get_interface_name(service)) << "]"
                      << endl;
             }
@@ -188,68 +188,68 @@
                             optind++;
                             data.writeStrongBinder(nullptr);
                         } else if (strcmp(argv[optind], "intent") == 0) {
-                        	
-                        	char* action = nullptr;
-                        	char* dataArg = nullptr;
-                        	char* type = nullptr;
-                        	int launchFlags = 0;
-                        	char* component = nullptr;
-                        	int categoryCount = 0;
-                        	char* categories[16];
-                        	
-                        	char* context1 = nullptr;
-                        	
+
+                            char* action = nullptr;
+                            char* dataArg = nullptr;
+                            char* type = nullptr;
+                            int launchFlags = 0;
+                            char* component = nullptr;
+                            int categoryCount = 0;
+                            char* categories[16];
+
+                            char* context1 = nullptr;
+
                             optind++;
-                            
-                        	while (optind < argc)
-                        	{
-                        		char* key = strtok_r(argv[optind], "=", &context1);
-                        		char* value = strtok_r(nullptr, "=", &context1);
-                                
+
+                            while (optind < argc)
+                            {
+                                char* key = strtok_r(argv[optind], "=", &context1);
+                                char* value = strtok_r(nullptr, "=", &context1);
+
                                 // we have reached the end of the XXX=XXX args.
                                 if (key == nullptr) break;
-                        		
-                        		if (strcmp(key, "action") == 0)
-                        		{
-                        			action = value;
-                        		}
-                        		else if (strcmp(key, "data") == 0)
-                        		{
-                        			dataArg = value;
-                        		}
-                        		else if (strcmp(key, "type") == 0)
-                        		{
-                        			type = value;
-                        		}
-                        		else if (strcmp(key, "launchFlags") == 0)
-                        		{
-                        			launchFlags = atoi(value);
-                        		}
-                        		else if (strcmp(key, "component") == 0)
-                        		{
-                        			component = value;
-                        		}
-                        		else if (strcmp(key, "categories") == 0)
-                        		{
-                        			char* context2 = nullptr;
-                        			categories[categoryCount] = strtok_r(value, ",", &context2);
-                        			
-                        			while (categories[categoryCount] != nullptr)
-                        			{
-                        				categoryCount++;
-                        				categories[categoryCount] = strtok_r(nullptr, ",", &context2);
-                        			}
-                        		}
-                                
+
+                                if (strcmp(key, "action") == 0)
+                                {
+                                    action = value;
+                                }
+                                else if (strcmp(key, "data") == 0)
+                                {
+                                    dataArg = value;
+                                }
+                                else if (strcmp(key, "type") == 0)
+                                {
+                                    type = value;
+                                }
+                                else if (strcmp(key, "launchFlags") == 0)
+                                {
+                                    launchFlags = atoi(value);
+                                }
+                                else if (strcmp(key, "component") == 0)
+                                {
+                                    component = value;
+                                }
+                                else if (strcmp(key, "categories") == 0)
+                                {
+                                    char* context2 = nullptr;
+                                    categories[categoryCount] = strtok_r(value, ",", &context2);
+
+                                    while (categories[categoryCount] != nullptr)
+                                    {
+                                        categoryCount++;
+                                        categories[categoryCount] = strtok_r(nullptr, ",", &context2);
+                                    }
+                                }
+
                                 optind++;
-                        	} 
-                        	
+                            }
+
                             writeString16(data, action);
                             writeString16(data, dataArg);
                             writeString16(data, type);
-                       		data.writeInt32(launchFlags);
+                            data.writeInt32(launchFlags);
                             writeString16(data, component);
-                        	
+
                             if (categoryCount > 0)
                             {
                                 data.writeInt32(categoryCount);
@@ -261,10 +261,10 @@
                             else
                             {
                                 data.writeInt32(0);
-                            }                            
-  
+                            }
+
                             // for now just set the extra field to be null.
-                       		data.writeInt32(-1);
+                            data.writeInt32(-1);
                         } else {
                             aerr << "service: unknown option " << argv[optind] << endl;
                             wantsUsage = true;
@@ -272,7 +272,7 @@
                             break;
                         }
                     }
-                    
+
                     service->transact(code, data, &reply);
                     aout << "Result: " << reply << endl;
                 } else {
@@ -295,7 +295,7 @@
             result = 10;
         }
     }
-    
+
     if (wantsUsage) {
         aout << "Usage: service [-h|-?]\n"
                 "       service list\n"
@@ -311,7 +311,7 @@
 //                "       action=STR data=STR type=STR launchFlags=INT component=STR categories=STR[,STR,...]\n";
         return result;
     }
-    
+
     return result;
 }
 
diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp
index b88b67d..6cfcf40 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -63,6 +63,21 @@
     return Status::ok();
 }
 
+bool isValidServiceName(const std::string& name) {
+    if (name.size() == 0) return false;
+    if (name.size() > 127) return false;
+
+    for (char c : name) {
+        if (c == '_' || c == '-' || c == '.') continue;
+        if (c >= 'a' && c <= 'z') continue;
+        if (c >= 'A' && c <= 'Z') continue;
+        if (c >= '0' && c <= '9') continue;
+        return false;
+    }
+
+    return true;
+}
+
 Status ServiceManager::addService(const std::string& name, const sp<IBinder>& binder, bool allowIsolated, int32_t dumpPriority) {
     auto ctx = mAccess->getCallingContext(name);
 
@@ -79,8 +94,8 @@
         return Status::fromExceptionCode(Status::EX_ILLEGAL_ARGUMENT);
     }
 
-    // match legacy rules
-    if (name.size() == 0 || name.size() > 127) {
+    if (!isValidServiceName(name)) {
+        LOG(ERROR) << "Invalid service name: " << name;
         return Status::fromExceptionCode(Status::EX_ILLEGAL_ARGUMENT);
     }
 
diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp
index 812d5ca..25d2aa8 100644
--- a/cmds/servicemanager/test_sm.cpp
+++ b/cmds/servicemanager/test_sm.cpp
@@ -82,6 +82,12 @@
         IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
 }
 
+TEST(AddService, WeirdCharactersDisallowed) {
+    auto sm = getPermissiveServiceManager();
+    EXPECT_FALSE(sm->addService("happy$foo$foo", getBinder(), false /*allowIsolated*/,
+        IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
+}
+
 TEST(AddService, AddNullServiceDisallowed) {
     auto sm = getPermissiveServiceManager();
     EXPECT_FALSE(sm->addService("foo", nullptr, false /*allowIsolated*/,
diff --git a/libs/binder/include/private/binder/ParcelValTypes.h b/libs/binder/ParcelValTypes.h
similarity index 100%
rename from libs/binder/include/private/binder/ParcelValTypes.h
rename to libs/binder/ParcelValTypes.h
diff --git a/libs/binder/PersistableBundle.cpp b/libs/binder/PersistableBundle.cpp
index c0aec0a..97a6c94 100644
--- a/libs/binder/PersistableBundle.cpp
+++ b/libs/binder/PersistableBundle.cpp
@@ -17,7 +17,6 @@
 #define LOG_TAG "PersistableBundle"
 
 #include <binder/PersistableBundle.h>
-#include <private/binder/ParcelValTypes.h>
 
 #include <limits>
 
@@ -26,6 +25,8 @@
 #include <log/log.h>
 #include <utils/Errors.h>
 
+#include "ParcelValTypes.h"
+
 using android::BAD_TYPE;
 using android::BAD_VALUE;
 using android::NO_ERROR;
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index edbe05e..44a691d 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -19,8 +19,6 @@
     cflags: [
         "-Wall",
         "-Werror",
-        "-Wno-unused-private-field",
-        "-Wno-unused-variable",
     ],
 }
 
diff --git a/libs/binder/tests/binderDriverInterfaceTest.cpp b/libs/binder/tests/binderDriverInterfaceTest.cpp
index 77ebac8..f3ed6a6 100644
--- a/libs/binder/tests/binderDriverInterfaceTest.cpp
+++ b/libs/binder/tests/binderDriverInterfaceTest.cpp
@@ -230,7 +230,6 @@
 }
 
 TEST_F(BinderDriverInterfaceTest, Transaction) {
-    binder_uintptr_t cookie = 1234;
     struct {
         uint32_t cmd1;
         struct binder_transaction_data arg1;
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 71751aa..e51bceb 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -971,9 +971,6 @@
 
 TEST_F(BinderLibTest, PropagateFlagSet)
 {
-    status_t ret;
-    Parcel data, reply;
-
     IPCThreadState::self()->clearPropagateWorkSource();
     IPCThreadState::self()->setCallingWorkSourceUid(100);
     EXPECT_EQ(true, IPCThreadState::self()->shouldPropagateWorkSource());
@@ -981,9 +978,6 @@
 
 TEST_F(BinderLibTest, PropagateFlagCleared)
 {
-    status_t ret;
-    Parcel data, reply;
-
     IPCThreadState::self()->setCallingWorkSourceUid(100);
     IPCThreadState::self()->clearPropagateWorkSource();
     EXPECT_EQ(false, IPCThreadState::self()->shouldPropagateWorkSource());
@@ -991,9 +985,6 @@
 
 TEST_F(BinderLibTest, PropagateFlagRestored)
 {
-    status_t ret;
-    Parcel data, reply;
-
     int token = IPCThreadState::self()->setCallingWorkSourceUid(100);
     IPCThreadState::self()->restoreCallingWorkSource(token);
 
@@ -1092,7 +1083,6 @@
             case BINDER_LIB_TEST_ADD_POLL_SERVER:
             case BINDER_LIB_TEST_ADD_SERVER: {
                 int ret;
-                uint8_t buf[1] = { 0 };
                 int serverid;
 
                 if (m_id != 0) {
@@ -1355,7 +1345,6 @@
         bool m_serverStartRequested;
         sp<IBinder> m_serverStarted;
         sp<IBinder> m_strongRef;
-        bool m_callbackPending;
         sp<IBinder> m_callback;
 };
 
@@ -1417,7 +1406,7 @@
               * We simulate a single-threaded process using the binder poll
               * interface; besides handling binder commands, it can also
               * issue outgoing transactions, by storing a callback in
-              * m_callback and setting m_callbackPending.
+              * m_callback.
               *
               * processPendingCall() will then issue that transaction.
               */
@@ -1444,8 +1433,6 @@
 }
 
 int main(int argc, char **argv) {
-    int ret;
-
     if (argc == 4 && !strcmp(argv[1], "--servername")) {
         binderservername = argv[2];
     } else {
diff --git a/libs/binder/tests/binderSafeInterfaceTest.cpp b/libs/binder/tests/binderSafeInterfaceTest.cpp
index 3b1db27..09f58cc 100644
--- a/libs/binder/tests/binderSafeInterfaceTest.cpp
+++ b/libs/binder/tests/binderSafeInterfaceTest.cpp
@@ -75,7 +75,7 @@
 
 private:
     int32_t mValue = 0;
-    uint8_t mPadding[4] = {}; // Avoids a warning from -Wpadded
+    __attribute__((unused)) uint8_t mPadding[4] = {}; // Avoids a warning from -Wpadded
 };
 
 struct TestFlattenable : Flattenable<TestFlattenable> {
@@ -743,6 +743,7 @@
     const std::vector<TestParcelable> a{TestParcelable{1}, TestParcelable{2}};
     std::vector<TestParcelable> aPlusOne;
     status_t result = mSafeInterfaceTest->increment(a, &aPlusOne);
+    ASSERT_EQ(NO_ERROR, result);
     ASSERT_EQ(a.size(), aPlusOne.size());
     for (size_t i = 0; i < a.size(); ++i) {
         ASSERT_EQ(a[i].getValue() + 1, aPlusOne[i].getValue());
diff --git a/libs/binder/tests/schd-dbg.cpp b/libs/binder/tests/schd-dbg.cpp
index ec9534a..ab4c56a 100644
--- a/libs/binder/tests/schd-dbg.cpp
+++ b/libs/binder/tests/schd-dbg.cpp
@@ -290,6 +290,7 @@
 
   sta = tickNow();
   status_t ret = workers[target]->transact(BINDER_NOP, data, &reply);
+  ASSERT(ret == NO_ERROR);
   end = tickNow();
   results_fifo->add_time(tickNano(sta, end));