Merge "libbinder: better object logs" into main
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 734b4b6..842ea77 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -1150,31 +1150,6 @@
     return NO_ERROR;
 }
 
-status_t Parcel::writeUnpadded(const void* data, size_t len)
-{
-    if (len > INT32_MAX) {
-        // don't accept size_t values which may have come from an
-        // inadvertent conversion from a negative int.
-        return BAD_VALUE;
-    }
-
-    size_t end = mDataPos + len;
-    if (end < mDataPos) {
-        // integer overflow
-        return BAD_VALUE;
-    }
-
-    if (end <= mDataCapacity) {
-restart_write:
-        memcpy(mData+mDataPos, data, len);
-        return finishWrite(len);
-    }
-
-    status_t err = growData(len);
-    if (err == NO_ERROR) goto restart_write;
-    return err;
-}
-
 status_t Parcel::write(const void* data, size_t len)
 {
     if (len > INT32_MAX) {
@@ -2951,6 +2926,14 @@
         return BAD_VALUE;
     }
 
+    if (mDataPos > mDataSize) {
+        // b/370831157 - this case used to abort. We also don't expect mDataPos < mDataSize, but
+        // this would only waste a bit of memory, so it's okay.
+        ALOGE("growData only expected at the end of a Parcel. pos: %zu, size: %zu, capacity: %zu",
+              mDataPos, len, mDataCapacity);
+        return BAD_VALUE;
+    }
+
     if (len > SIZE_MAX - mDataSize) return NO_MEMORY; // overflow
     if (mDataSize + len > SIZE_MAX / 3) return NO_MEMORY; // overflow
     size_t newSize = ((mDataSize+len)*3)/2;
diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h
index e2b23be..ed4e211 100644
--- a/libs/binder/include/binder/Parcel.h
+++ b/libs/binder/include/binder/Parcel.h
@@ -172,7 +172,6 @@
 
     LIBBINDER_EXPORTED status_t write(const void* data, size_t len);
     LIBBINDER_EXPORTED void* writeInplace(size_t len);
-    LIBBINDER_EXPORTED status_t writeUnpadded(const void* data, size_t len);
     LIBBINDER_EXPORTED status_t writeInt32(int32_t val);
     LIBBINDER_EXPORTED status_t writeUint32(uint32_t val);
     LIBBINDER_EXPORTED status_t writeInt64(int64_t val);
@@ -637,9 +636,6 @@
 
     LIBBINDER_EXPORTED const flat_binder_object* readObject(bool nullMetaData) const;
 
-    // Explicitly close all file descriptors in the parcel.
-    LIBBINDER_EXPORTED void closeFileDescriptors();
-
     // Debugging: get metrics on current allocations.
     LIBBINDER_EXPORTED static size_t getGlobalAllocSize();
     LIBBINDER_EXPORTED static size_t getGlobalAllocCount();
@@ -652,6 +648,9 @@
     LIBBINDER_EXPORTED void print(std::ostream& to, uint32_t flags = 0) const;
 
 private:
+    // Explicitly close all file descriptors in the parcel.
+    void closeFileDescriptors();
+
     // `objects` and `objectsSize` always 0 for RPC Parcels.
     typedef void (*release_func)(const uint8_t* data, size_t dataSize, const binder_size_t* objects,
                                  size_t objectsSize);
diff --git a/libs/binder/ndk/binder_rpc.cpp b/libs/binder/ndk/binder_rpc.cpp
index 2cc5f81..3c32a39 100644
--- a/libs/binder/ndk/binder_rpc.cpp
+++ b/libs/binder/ndk/binder_rpc.cpp
@@ -94,7 +94,7 @@
     }
     void* mData;
     ABinderRpc_AccessorProviderUserData_deleteCallback mOnDelete;
-    // needs to be copyable for std::function, but we will never copy it
+    // needs to be copy-able for std::function, but we will never copy it
     OnDeleteProviderHolder(const OnDeleteProviderHolder&) {
         LOG_ALWAYS_FATAL("This object can't be copied!");
     }
@@ -113,7 +113,7 @@
     }
     if (data && onDelete == nullptr) {
         ALOGE("If a non-null data ptr is passed to ABinderRpc_registerAccessorProvider, then a "
-              "ABinderRpc_AccessorProviderUserData_deleteCallback must alse be passed to delete "
+              "ABinderRpc_AccessorProviderUserData_deleteCallback must also be passed to delete "
               "the data object once the ABinderRpc_AccessorProvider is removed.");
         return nullptr;
     }
@@ -179,7 +179,7 @@
     }
     void* mData;
     ABinderRpc_ConnectionInfoProviderUserData_delete mOnDelete;
-    // needs to be copyable for std::function, but we will never copy it
+    // needs to be copy-able for std::function, but we will never copy it
     OnDeleteConnectionInfoHolder(const OnDeleteConnectionInfoHolder&) {
         LOG_ALWAYS_FATAL("This object can't be copied!");
     }
@@ -197,7 +197,7 @@
     }
     if (data && onDelete == nullptr) {
         ALOGE("If a non-null data ptr is passed to ABinderRpc_Accessor_new, then a "
-              "ABinderRpc_ConnectionInfoProviderUserData_delete callback must alse be passed to "
+              "ABinderRpc_ConnectionInfoProviderUserData_delete callback must also be passed to "
               "delete "
               "the data object once the ABinderRpc_Accessor is deleted.");
         return nullptr;
@@ -304,7 +304,7 @@
 
 ABinderRpc_ConnectionInfo* ABinderRpc_ConnectionInfo_new(const sockaddr* addr, socklen_t len) {
     if (addr == nullptr || len < 0 || static_cast<size_t>(len) < sizeof(sa_family_t)) {
-        ALOGE("Invalid arguments in Arpc_Connection_new");
+        ALOGE("Invalid arguments in ABinderRpc_Connection_new");
         return nullptr;
     }
     // socklen_t was int32_t on 32-bit and uint32_t on 64 bit.
@@ -317,8 +317,9 @@
             return nullptr;
         }
         sockaddr_vm vm = *reinterpret_cast<const sockaddr_vm*>(addr);
-        LOG_ACCESSOR_DEBUG("Arpc_ConnectionInfo_new found AF_VSOCK. family %d, port %d, cid %d",
-                           vm.svm_family, vm.svm_port, vm.svm_cid);
+        LOG_ACCESSOR_DEBUG(
+                "ABinderRpc_ConnectionInfo_new found AF_VSOCK. family %d, port %d, cid %d",
+                vm.svm_family, vm.svm_port, vm.svm_cid);
         return new ABinderRpc_ConnectionInfo(vm);
     } else if (addr->sa_family == AF_UNIX) {
         if (len != sizeof(sockaddr_un)) {
@@ -327,7 +328,7 @@
             return nullptr;
         }
         sockaddr_un un = *reinterpret_cast<const sockaddr_un*>(addr);
-        LOG_ACCESSOR_DEBUG("Arpc_ConnectionInfo_new found AF_UNIX. family %d, path %s",
+        LOG_ACCESSOR_DEBUG("ABinderRpc_ConnectionInfo_new found AF_UNIX. family %d, path %s",
                            un.sun_family, un.sun_path);
         return new ABinderRpc_ConnectionInfo(un);
     } else if (addr->sa_family == AF_INET) {
@@ -337,12 +338,14 @@
             return nullptr;
         }
         sockaddr_in in = *reinterpret_cast<const sockaddr_in*>(addr);
-        LOG_ACCESSOR_DEBUG("Arpc_ConnectionInfo_new found AF_INET. family %d, address %s, port %d",
-                           in.sin_family, inet_ntoa(in.sin_addr), ntohs(in.sin_port));
+        LOG_ACCESSOR_DEBUG(
+                "ABinderRpc_ConnectionInfo_new found AF_INET. family %d, address %s, port %d",
+                in.sin_family, inet_ntoa(in.sin_addr), ntohs(in.sin_port));
         return new ABinderRpc_ConnectionInfo(in);
     }
 
-    ALOGE("ARpc APIs only support AF_VSOCK right now but the supplied sockadder::sa_family is: %hu",
+    ALOGE("ABinderRpc APIs only support AF_VSOCK right now but the supplied sockaddr::sa_family "
+          "is: %hu",
           addr->sa_family);
     return nullptr;
 }
diff --git a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
index acb6970..8296356 100644
--- a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
@@ -298,8 +298,7 @@
 #endif
 
 // TODO(b/368559337): fix versioning on product partition
-// TODO(b/370091328): APEX modules call this function even it is unavailable.
-#if !defined(__ANDROID_APEX__) && !defined(__ANDROID_PRODUCT__) && \
+#if !defined(__ANDROID_PRODUCT__) && \
         (defined(__ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__) || __ANDROID_API__ >= 36)
     if API_LEVEL_AT_LEAST (36, 202504) {
         if (codeToFunction != nullptr) {
@@ -310,8 +309,7 @@
 #else
     (void)codeToFunction;
     (void)functionCount;
-#endif  // !defined(__ANDROID_APEX__) && !defined(__ANDROID_PRODUCT__) && \
-//     (defined(__ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__) || __ANDROID_API__ >= 36)
+#endif  // defined(__ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__) || __ANDROID_API__ >= 36
     return clazz;
 }
 
diff --git a/libs/binder/ndk/include_ndk/android/binder_ibinder.h b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
index 2f6c4e3..bd46c47 100644
--- a/libs/binder/ndk/include_ndk/android/binder_ibinder.h
+++ b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
@@ -224,8 +224,10 @@
  *
  * Trace messages will use the provided names instead of bare integer codes when set. If not set by
  * this function, trace messages will only be identified by the bare code. This should be called one
- * time during clazz initialization. clazz and transactionCodeToFunctionMap should have same
- * lifetime. Resetting/clearing the transactionCodeToFunctionMap is not allowed.
+ * time during clazz initialization. clazz is defined using AIBinder_Class_define and
+ * transactionCodeToFunctionMap should have same scope as clazz. Resetting/clearing the
+ * transactionCodeToFunctionMap is not allowed. Passing null for either clazz or
+ * transactionCodeToFunctionMap will abort.
  *
  * Available since API level 36.
  *
@@ -236,9 +238,6 @@
  * contiguous, and this is required for maximum memory efficiency.
  * You can use nullptr if certain transaction codes are not used. Lifetime should be same as clazz.
  * \param length number of elements in the transactionCodeToFunctionMap
- *
- * \return true if setting codeToFunction to clazz is successful. return false if clazz or
- * codeToFunction is nullptr.
  */
 void AIBinder_Class_setTransactionCodeToFunctionNameMap(AIBinder_Class* clazz,
                                                         const char** transactionCodeToFunctionMap,
@@ -257,7 +256,8 @@
  * \param transactionCode transaction_code_t for which function name is requested.
  *
  * \return function name in form of const char* if transaction code is valid for given class.
- * if transaction code is invalid or transactionCodeToFunctionMap is not set, nullptr is returned
+ * The value returned is valid for the lifetime of clazz. if transaction code is invalid or
+ * transactionCodeToFunctionMap is not set, nullptr is returned.
  */
 const char* AIBinder_Class_getFunctionName(AIBinder_Class* clazz, transaction_code_t code)
         __INTRODUCED_IN(36);
diff --git a/libs/binder/ndk/include_platform/android/binder_rpc.h b/libs/binder/ndk/include_platform/android/binder_rpc.h
index 4c5471f..7132554 100644
--- a/libs/binder/ndk/include_platform/android/binder_rpc.h
+++ b/libs/binder/ndk/include_platform/android/binder_rpc.h
@@ -22,6 +22,22 @@
 __BEGIN_DECLS
 
 /**
+ * @defgroup ABinderRpc Binder RPC
+ *
+ * This set of APIs makes it possible for a process to use the AServiceManager
+ * APIs to get binder objects for services that are available over sockets
+ * instead of the traditional kernel binder with the extra ServiceManager
+ * process.
+ *
+ * These APIs are used to supply libbinder with enough information to create
+ * and manage the socket connections underneath the ServiceManager APIs so the
+ * clients do not need to know the service implementation details or what
+ * transport they use for communication.
+ *
+ * @{
+ */
+
+/**
  * This represents an IAccessor implementation from libbinder that is
  * responsible for providing a pre-connected socket file descriptor for a
  * specific service. The service is an RpcServer and the pre-connected socket is
@@ -66,7 +82,8 @@
  * libbinder.
  *
  * \param instance name of the service like
- *        `android.hardware.vibrator.IVibrator/default`
+ *        `android.hardware.vibrator.IVibrator/default`. This string must remain
+ *        valid and unchanged for the duration of this function call.
  * \param data the data that was associated with this instance when the callback
  *        was registered.
  * \return The ABinderRpc_Accessor associated with the service `instance`. This
@@ -103,13 +120,15 @@
  *        instance is being registered that was previously registered, this call
  *        will fail and the ABinderRpc_AccessorProviderUserData_deleteCallback
  *        will be called to clean up the data.
+ *        This array of strings must remain valid and unchanged for the duration
+ *        of this function call.
  * \param number of instances in the instances array.
  * \param data pointer that is passed to the ABinderRpc_AccessorProvider callback.
  *        IMPORTANT: The ABinderRpc_AccessorProvider now OWNS that object that data
  *        points to. It can be used as necessary in the callback. The data MUST
  *        remain valid for the lifetime of the provider callback.
  *        Do not attempt to give ownership of the same object to different
- *        providers throguh multiple calls to this function because the first
+ *        providers through multiple calls to this function because the first
  *        one to be deleted will call the onDelete callback.
  * \param onDelete callback used to delete the objects that `data` points to.
  *        This is called after ABinderRpc_AccessorProvider is guaranteed to never be
@@ -151,8 +170,9 @@
  * connect to a socket that a given service is listening on. This is needed to
  * create an ABinderRpc_Accessor so it can connect to these services.
  *
- * \param instance name of the service to connect to
- * \param data userdata for this callback. The pointer is provided in
+ * \param instance name of the service to connect to. This string must remain
+ *        valid and unchanged for the duration of this function call.
+ * \param data user data for this callback. The pointer is provided in
  *        ABinderRpc_Accessor_new.
  * \return ABinderRpc_ConnectionInfo with socket connection information for `instance`
  */
@@ -177,7 +197,9 @@
  * that can use the info from the ABinderRpc_ConnectionInfoProvider to connect to a
  * socket that the service with `instance` name is listening to.
  *
- * \param instance name of the service that is listening on the socket
+ * \param instance name of the service that is listening on the socket. This
+ *        string must remain valid and unchanged for the duration of this
+ *        function call.
  * \param provider callback that can get the socket connection information for the
  *           instance. This connection information may be dynamic, so the
  *           provider will be called any time a new connection is required.
@@ -219,18 +241,24 @@
 
 /**
  * Return the ABinderRpc_Accessor associated with an AIBinder. The instance must match
- * the ABinderRpc_Accessor implementation, and the AIBinder must a proxy binder for a
- * remote service (ABpBinder).
- * This can be used when receivng an AIBinder from another process that the
+ * the ABinderRpc_Accessor implementation.
+ * This can be used when receiving an AIBinder from another process that the
  * other process obtained from ABinderRpc_Accessor_asBinder.
  *
  * \param instance name of the service that the Accessor is responsible for.
- * \param accessorBinder proxy binder from another processes ABinderRpc_Accessor.
+ *        This string must remain valid and unchanged for the duration of this
+ *        function call.
+ * \param accessorBinder proxy binder from another process's ABinderRpc_Accessor.
+ *        This function preserves the refcount of this binder object and the
+ *        caller still owns it.
  * \return ABinderRpc_Accessor representing the other processes ABinderRpc_Accessor
- *         implementation. This function does not take ownership of the
- *         ABinderRpc_Accessor (so the caller needs to delete with
- *         ABinderRpc_Accessor_delete), and it preserves the recount of the bidner
- *         object.
+ *         implementation. The caller owns this ABinderRpc_Accessor instance and
+ *         is responsible for deleting it with ABinderRpc_Accessor_delete or
+ *         passing ownership of it elsewhere, like returning it through
+ *         ABinderRpc_AccessorProvider_getAccessorCallback.
+ *         nullptr on error when the accessorBinder is not a valid binder from
+ *         an IAccessor implementation or the IAccessor implementation is not
+ *         associated with the provided instance.
  */
 ABinderRpc_Accessor* _Nullable ABinderRpc_Accessor_fromBinder(const char* _Nonnull instance,
                                                               AIBinder* _Nonnull accessorBinder)
@@ -258,4 +286,6 @@
  */
 void ABinderRpc_ConnectionInfo_delete(ABinderRpc_ConnectionInfo* _Nonnull info) __INTRODUCED_IN(36);
 
+/** @} */
+
 __END_DECLS
diff --git a/libs/binder/tests/binderCacheUnitTest.cpp b/libs/binder/tests/binderCacheUnitTest.cpp
index 482d197..be5d559 100644
--- a/libs/binder/tests/binderCacheUnitTest.cpp
+++ b/libs/binder/tests/binderCacheUnitTest.cpp
@@ -149,7 +149,16 @@
     EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder2));
 
     // Confirm that new service is returned instead of old.
-    sp<IBinder> result2 = mServiceManager->checkService(kCachedServiceName);
+    int retry_count = 5;
+    sp<IBinder> result2;
+    do {
+        std::this_thread::sleep_for(std::chrono::milliseconds(50));
+        if (retry_count-- == 0) {
+            break;
+        }
+        result2 = mServiceManager->checkService(kCachedServiceName);
+    } while (result2 != binder2);
+
     ASSERT_EQ(binder2, result2);
 }
 
diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp
index 3337c47..4b43c27 100644
--- a/services/inputflinger/dispatcher/InputDispatcher.cpp
+++ b/services/inputflinger/dispatcher/InputDispatcher.cpp
@@ -1030,7 +1030,7 @@
         const nsecs_t nextAnrCheck = processAnrsLocked();
         nextWakeupTime = std::min(nextWakeupTime, nextAnrCheck);
 
-        if (input_flags::enable_per_device_input_latency_metrics()) {
+        if (mPerDeviceInputLatencyMetricsFlag) {
             const nsecs_t nextStatisticsPush = processLatencyStatisticsLocked();
             nextWakeupTime = std::min(nextWakeupTime, nextStatisticsPush);
         }
@@ -4549,7 +4549,7 @@
             newEntry->traceTracker = mTracer->traceInboundEvent(*newEntry);
         }
 
-        if (input_flags::enable_per_device_input_latency_metrics()) {
+        if (mPerDeviceInputLatencyMetricsFlag) {
             if (args.id != android::os::IInputConstants::INVALID_INPUT_EVENT_ID &&
                 IdGenerator::getSource(args.id) == IdGenerator::Source::INPUT_READER &&
                 !mInputFilterEnabled) {
diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h
index d90b9de..24e36ae 100644
--- a/services/inputflinger/dispatcher/InputDispatcher.h
+++ b/services/inputflinger/dispatcher/InputDispatcher.h
@@ -16,6 +16,8 @@
 
 #pragma once
 
+#include <com_android_input_flags.h>
+
 #include "AnrTracker.h"
 #include "CancelationOptions.h"
 #include "DragState.h"
@@ -741,6 +743,10 @@
 
     sp<android::gui::WindowInfoHandle> findWallpaperWindowBelow(
             const sp<android::gui::WindowInfoHandle>& windowHandle) const REQUIRES(mLock);
+
+    /** Stores the value of the input flag for per device input latency metrics. */
+    const bool mPerDeviceInputLatencyMetricsFlag =
+            com::android::input::flags::enable_per_device_input_latency_metrics();
 };
 
 } // namespace android::inputdispatcher
diff --git a/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp b/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp
index 486d893..157bee3 100644
--- a/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp
+++ b/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp
@@ -103,7 +103,7 @@
                 .WillRepeatedly(Return(false));
     }
 
-    std::map<const char*, int64_t> mTelemetryLogCounts;
+    std::map<std::string, int64_t> mTelemetryLogCounts;
 
     /**
      * A fake function for telemetry logging.
diff --git a/services/surfaceflinger/Display/DisplayModeController.cpp b/services/surfaceflinger/Display/DisplayModeController.cpp
index 0e9218c..f8b6c6e 100644
--- a/services/surfaceflinger/Display/DisplayModeController.cpp
+++ b/services/surfaceflinger/Display/DisplayModeController.cpp
@@ -283,6 +283,8 @@
     }
 }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-value" // b/369277774
 auto DisplayModeController::getKernelIdleTimerState(PhysicalDisplayId displayId) const
         -> KernelIdleTimerState {
     std::lock_guard lock(mDisplayLock);
@@ -298,4 +300,5 @@
     return {desiredModeIdOpt, displayPtr->isKernelIdleTimerEnabled};
 }
 
+#pragma clang diagnostic pop
 } // namespace android::display