From 88b84ec200091bdf5754b435ef55dfcd30078a67 Mon Sep 17 00:00:00 2001 From: Ningsheng Jian Date: Wed, 17 Sep 2014 13:34:09 +0800 Subject: NativeHelper: Avoid returning local stack string Refactor JniInvocation::GetLibrary to not return a stack-allocated string. Instead, provide a char buffer. Bug: 16404669 Change-Id: I34f4a40e28bc491ba630a2b1bff5792e34937101 --- Android.mk | 6 ++++++ JniInvocation.cpp | 26 ++++++++++++++++++++------ include/nativehelper/JniInvocation.h | 7 +++++-- tests/Android.mk | 18 ++---------------- tests/JniInvocation_test.cpp | 17 +++++++++++++---- 5 files changed, 46 insertions(+), 28 deletions(-) diff --git a/Android.mk b/Android.mk index 938d459..59839d4 100644 --- a/Android.mk +++ b/Android.mk @@ -82,3 +82,9 @@ LOCAL_LDFLAGS := -ldl LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk LOCAL_MULTILIB := both include $(BUILD_HOST_SHARED_LIBRARY) + +# +# Tests. +# + +include $(LOCAL_PATH)/tests/Android.mk diff --git a/JniInvocation.cpp b/JniInvocation.cpp index f4dd24e..30b1205 100644 --- a/JniInvocation.cpp +++ b/JniInvocation.cpp @@ -55,9 +55,11 @@ static const char* kDebuggableFallback = "0"; // Not debuggable. #endif static const char* kLibraryFallback = "libart.so"; -const char* JniInvocation::GetLibrary(const char* library) { +template void UNUSED(const T&) {} + +const char* JniInvocation::GetLibrary(const char* library, char* buffer) { #ifdef HAVE_ANDROID_OS - char default_library[PROPERTY_VALUE_MAX]; + const char* default_library; char debuggable[PROPERTY_VALUE_MAX]; property_get(kDebuggableSystemProperty, debuggable, kDebuggableFallback); @@ -65,17 +67,24 @@ const char* JniInvocation::GetLibrary(const char* library) { if (strcmp(debuggable, "1") != 0) { // Not a debuggable build. // Do not allow arbitrary library. Ignore the library parameter. This - // will also ignore the default library, but initialize to empty string + // will also ignore the default library, but initialize to fallback // for cleanliness. library = kLibraryFallback; - default_library[0] = 0; + default_library = kLibraryFallback; } else { // Debuggable build. // Accept the library parameter. For the case it is NULL, load the default // library from the system property. - property_get(kLibrarySystemProperty, default_library, kLibraryFallback); + if (buffer != NULL) { + property_get(kLibrarySystemProperty, buffer, kLibraryFallback); + default_library = buffer; + } else { + // No buffer given, just use default fallback. + default_library = kLibraryFallback; + } } #else + UNUSED(buffer); const char* default_library = kLibraryFallback; #endif if (library == NULL) { @@ -86,7 +95,12 @@ const char* JniInvocation::GetLibrary(const char* library) { } bool JniInvocation::Init(const char* library) { - library = GetLibrary(library); +#ifdef HAVE_ANDROID_OS + char buffer[PROPERTY_VALUE_MAX]; +#else + char* buffer = NULL; +#endif + library = GetLibrary(library, buffer); handle_ = dlopen(library, RTLD_NOW); if (handle_ == NULL) { diff --git a/include/nativehelper/JniInvocation.h b/include/nativehelper/JniInvocation.h index b5198ff..fc2ed0a 100644 --- a/include/nativehelper/JniInvocation.h +++ b/include/nativehelper/JniInvocation.h @@ -38,8 +38,11 @@ class JniInvocation { // persist.sys.dalvik.vm.lib. bool Init(const char* library); - // Exposes which library is actually loaded from the given name. - static const char* GetLibrary(const char* library); + // Exposes which library is actually loaded from the given name. The + // buffer of size PROPERTY_VALUE_MAX will be used to load the system + // property for the default library, if necessary. If no buffer is + // provided, the fallback value will be used. + static const char* GetLibrary(const char* library, char* buffer); private: diff --git a/tests/Android.mk b/tests/Android.mk index c357b24..b40bdf8 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -8,14 +8,7 @@ include $(CLEAR_VARS) LOCAL_MODULE := JniInvocation_test LOCAL_CLANG := true LOCAL_SRC_FILES := JniInvocation_test.cpp -LOCAL_SHARED_LIBRARIES := \ - libnativehelper - -include external/libcxx/libcxx.mk - -LOCAL_MULTILIB := both -LOCAL_MODULE_STEM_32 := $(LOCAL_MODULE)32 -LOCAL_MODULE_STEM_64 := $(LOCAL_MODULE)64 +LOCAL_SHARED_LIBRARIES := libnativehelper include $(BUILD_NATIVE_TEST) # Host unit test. @@ -24,12 +17,5 @@ include $(CLEAR_VARS) LOCAL_MODULE := JniInvocation_test LOCAL_CLANG := true LOCAL_SRC_FILES := JniInvocation_test.cpp -LOCAL_SHARED_LIBRARIES := \ - libnativehelper - -include external/libcxx/libcxx.mk - -LOCAL_MULTILIB := both -LOCAL_MODULE_STEM_32 := $(LOCAL_MODULE)32 -LOCAL_MODULE_STEM_64 := $(LOCAL_MODULE)64 +LOCAL_SHARED_LIBRARIES := libnativehelper include $(BUILD_HOST_NATIVE_TEST) diff --git a/tests/JniInvocation_test.cpp b/tests/JniInvocation_test.cpp index 6eb0267..b1d2b68 100644 --- a/tests/JniInvocation_test.cpp +++ b/tests/JniInvocation_test.cpp @@ -30,6 +30,9 @@ #ifdef HAVE_TEST_STUFF +// PROPERTY_VALUE_MAX. +#include "cutils/properties.h" + // Ability to have fake local system properties. #define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ #include @@ -102,19 +105,22 @@ TEST_F(JNIInvocationTest, Debuggable) { ASSERT_EQ(0, __system_property_add(kDebuggableSystemProperty, 13, kIsDebuggableValue, 1)); ASSERT_EQ(0, __system_property_add(kLibrarySystemProperty, 27, kTestNonNull2, 11)); - const char* result = JniInvocation::GetLibrary(NULL); + char buffer[PROPERTY_VALUE_MAX]; + const char* result = JniInvocation::GetLibrary(NULL, buffer); EXPECT_FALSE(result == NULL); if (result != NULL) { EXPECT_TRUE(strcmp(result, kTestNonNull2) == 0); EXPECT_FALSE(strcmp(result, kExpected) == 0); } - result = JniInvocation::GetLibrary(kTestNonNull); + result = JniInvocation::GetLibrary(kTestNonNull, buffer); EXPECT_FALSE(result == NULL); if (result != NULL) { EXPECT_TRUE(strcmp(result, kTestNonNull) == 0); EXPECT_FALSE(strcmp(result, kTestNonNull2) == 0); } +#else + GTEST_LOG_(WARNING) << "Host testing unsupported. Please run target tests."; #endif } @@ -124,7 +130,8 @@ TEST_F(JNIInvocationTest, NonDebuggable) { ASSERT_TRUE(pa.valid); ASSERT_EQ(0, __system_property_add(kDebuggableSystemProperty, 13, kIsNotDebuggableValue, 1)); - const char* result = JniInvocation::GetLibrary(NULL); + char buffer[PROPERTY_VALUE_MAX]; + const char* result = JniInvocation::GetLibrary(NULL, buffer); EXPECT_FALSE(result == NULL); if (result != NULL) { EXPECT_TRUE(strcmp(result, kExpected) == 0); @@ -132,12 +139,14 @@ TEST_F(JNIInvocationTest, NonDebuggable) { EXPECT_FALSE(strcmp(result, kTestNonNull2) == 0); } - result = JniInvocation::GetLibrary(kTestNonNull); + result = JniInvocation::GetLibrary(kTestNonNull, buffer); EXPECT_FALSE(result == NULL); if (result != NULL) { EXPECT_TRUE(strcmp(result, kExpected) == 0); EXPECT_FALSE(strcmp(result, kTestNonNull) == 0); } +#else + GTEST_LOG_(WARNING) << "Host testing unsupported. Please run target tests."; #endif } -- cgit v1.2.3