Merge "Clean up some resources code" (bfc18c1c) · Commits · e / os / android_frameworks_base

cmds/idmap2/libidmap2/Idmap.cpp

+1 −2

Original line number	Diff line number	Diff line
		@@ -77,8 +77,7 @@ bool WARN_UNUSED ReadString(std::istream& stream, std::string* out) {
		return false;
		}
		uint32_t padding_size = CalculatePadding(size);
		std::string padding(padding_size, '\0');
		if (!stream.read(padding.data(), padding_size)) {
		if (padding_size != 0 && !stream.seekg(padding_size, std::ios_base::cur)) {
		return false;
		}
		*out = buf;

cmds/idmap2/tests/Idmap2BinaryTests.cpp

+40 −41

Original line number	Diff line number	Diff line
		@@ -46,7 +46,6 @@

		using ::android::base::StringPrintf;
		using ::android::util::ExecuteBinary;
		using ::testing::NotNull;

		namespace android::idmap2 {

		@@ -95,8 +94,8 @@ TEST_F(Idmap2BinaryTests, Create) {
		"--overlay-name", TestConstants::OVERLAY_NAME_DEFAULT,
		"--idmap-path", GetIdmapPath()});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_EQ(result->status, EXIT_SUCCESS) << result->stderr_str;
		ASSERT_TRUE((bool)result);
		ASSERT_EQ(result.status, EXIT_SUCCESS) << result.stderr_str;

		struct stat st;
		ASSERT_EQ(stat(GetIdmapPath().c_str(), &st), 0);
		@@ -122,33 +121,33 @@ TEST_F(Idmap2BinaryTests, Dump) {
		"--overlay-name", TestConstants::OVERLAY_NAME_DEFAULT,
		"--idmap-path", GetIdmapPath()});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_EQ(result->status, EXIT_SUCCESS) << result->stderr_str;
		ASSERT_TRUE((bool)result);
		ASSERT_EQ(result.status, EXIT_SUCCESS) << result.stderr_str;

		// clang-format off
		result = ExecuteBinary({"idmap2",
		"dump",
		"--idmap-path", GetIdmapPath()});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_EQ(result->status, EXIT_SUCCESS) << result->stderr_str;
		ASSERT_TRUE((bool)result);
		ASSERT_EQ(result.status, EXIT_SUCCESS) << result.stderr_str;

		ASSERT_NE(result->stdout_str.find(StringPrintf("0x%08x -> 0x%08x", R::target::integer::int1,
		ASSERT_NE(result.stdout_str.find(StringPrintf("0x%08x -> 0x%08x", R::target::integer::int1,
		R::overlay::integer::int1)),
		std::string::npos)
		<< result->stdout_str;
		ASSERT_NE(result->stdout_str.find(StringPrintf("0x%08x -> 0x%08x", R::target::string::str1,
		<< result.stdout_str;
		ASSERT_NE(result.stdout_str.find(StringPrintf("0x%08x -> 0x%08x", R::target::string::str1,
		R::overlay::string::str1)),
		std::string::npos)
		<< result->stdout_str;
		ASSERT_NE(result->stdout_str.find(StringPrintf("0x%08x -> 0x%08x", R::target::string::str3,
		<< result.stdout_str;
		ASSERT_NE(result.stdout_str.find(StringPrintf("0x%08x -> 0x%08x", R::target::string::str3,
		R::overlay::string::str3)),
		std::string::npos)
		<< result->stdout_str;
		ASSERT_NE(result->stdout_str.find(StringPrintf("0x%08x -> 0x%08x", R::target::string::str4,
		<< result.stdout_str;
		ASSERT_NE(result.stdout_str.find(StringPrintf("0x%08x -> 0x%08x", R::target::string::str4,
		R::overlay::string::str4)),
		std::string::npos)
		<< result->stdout_str;
		<< result.stdout_str;

		// clang-format off
		result = ExecuteBinary({"idmap2",
		@@ -156,9 +155,9 @@ TEST_F(Idmap2BinaryTests, Dump) {
		"--verbose",
		"--idmap-path", GetIdmapPath()});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_EQ(result->status, EXIT_SUCCESS) << result->stderr_str;
		ASSERT_NE(result->stdout_str.find("00000000: 504d4449 magic"), std::string::npos);
		ASSERT_TRUE((bool)result);
		ASSERT_EQ(result.status, EXIT_SUCCESS) << result.stderr_str;
		ASSERT_NE(result.stdout_str.find("00000000: 504d4449 magic"), std::string::npos);

		// clang-format off
		result = ExecuteBinary({"idmap2",
		@@ -166,8 +165,8 @@ TEST_F(Idmap2BinaryTests, Dump) {
		"--verbose",
		"--idmap-path", GetTestDataPath() + "/DOES-NOT-EXIST"});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_NE(result->status, EXIT_SUCCESS);
		ASSERT_TRUE((bool)result);
		ASSERT_NE(result.status, EXIT_SUCCESS);

		unlink(GetIdmapPath().c_str());
		}
		@@ -183,8 +182,8 @@ TEST_F(Idmap2BinaryTests, Lookup) {
		"--overlay-name", TestConstants::OVERLAY_NAME_DEFAULT,
		"--idmap-path", GetIdmapPath()});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_EQ(result->status, EXIT_SUCCESS) << result->stderr_str;
		ASSERT_TRUE((bool)result);
		ASSERT_EQ(result.status, EXIT_SUCCESS) << result.stderr_str;

		// clang-format off
		result = ExecuteBinary({"idmap2",
		@@ -193,10 +192,10 @@ TEST_F(Idmap2BinaryTests, Lookup) {
		"--config", "",
		"--resid", StringPrintf("0x%08x", R::target::string::str1)});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_EQ(result->status, EXIT_SUCCESS) << result->stderr_str;
		ASSERT_NE(result->stdout_str.find("overlay-1"), std::string::npos);
		ASSERT_EQ(result->stdout_str.find("overlay-1-sv"), std::string::npos);
		ASSERT_TRUE((bool)result);
		ASSERT_EQ(result.status, EXIT_SUCCESS) << result.stderr_str;
		ASSERT_NE(result.stdout_str.find("overlay-1"), std::string::npos);
		ASSERT_EQ(result.stdout_str.find("overlay-1-sv"), std::string::npos);

		// clang-format off
		result = ExecuteBinary({"idmap2",
		@@ -205,10 +204,10 @@ TEST_F(Idmap2BinaryTests, Lookup) {
		"--config", "",
		"--resid", "test.target:string/str1"});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_EQ(result->status, EXIT_SUCCESS) << result->stderr_str;
		ASSERT_NE(result->stdout_str.find("overlay-1"), std::string::npos);
		ASSERT_EQ(result->stdout_str.find("overlay-1-sv"), std::string::npos);
		ASSERT_TRUE((bool)result);
		ASSERT_EQ(result.status, EXIT_SUCCESS) << result.stderr_str;
		ASSERT_NE(result.stdout_str.find("overlay-1"), std::string::npos);
		ASSERT_EQ(result.stdout_str.find("overlay-1-sv"), std::string::npos);

		// clang-format off
		result = ExecuteBinary({"idmap2",
		@@ -217,9 +216,9 @@ TEST_F(Idmap2BinaryTests, Lookup) {
		"--config", "sv",
		"--resid", "test.target:string/str1"});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_EQ(result->status, EXIT_SUCCESS) << result->stderr_str;
		ASSERT_NE(result->stdout_str.find("overlay-1-sv"), std::string::npos);
		ASSERT_TRUE((bool)result);
		ASSERT_EQ(result.status, EXIT_SUCCESS) << result.stderr_str;
		ASSERT_NE(result.stdout_str.find("overlay-1-sv"), std::string::npos);

		unlink(GetIdmapPath().c_str());
		}
		@@ -234,8 +233,8 @@ TEST_F(Idmap2BinaryTests, InvalidCommandLineOptions) {
		auto result = ExecuteBinary({"idmap2",
		"create"});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_NE(result->status, EXIT_SUCCESS);
		ASSERT_TRUE((bool)result);
		ASSERT_NE(result.status, EXIT_SUCCESS);

		// missing argument to option
		// clang-format off
		@@ -246,8 +245,8 @@ TEST_F(Idmap2BinaryTests, InvalidCommandLineOptions) {
		"--overlay-name", TestConstants::OVERLAY_NAME_DEFAULT,
		"--idmap-path"});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_NE(result->status, EXIT_SUCCESS);
		ASSERT_TRUE((bool)result);
		ASSERT_NE(result.status, EXIT_SUCCESS);

		// invalid target apk path
		// clang-format off
		@@ -258,8 +257,8 @@ TEST_F(Idmap2BinaryTests, InvalidCommandLineOptions) {
		"--overlay-name", TestConstants::OVERLAY_NAME_DEFAULT,
		"--idmap-path", GetIdmapPath()});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_NE(result->status, EXIT_SUCCESS);
		ASSERT_TRUE((bool)result);
		ASSERT_NE(result.status, EXIT_SUCCESS);

		// unknown policy
		// clang-format off
		@@ -271,8 +270,8 @@ TEST_F(Idmap2BinaryTests, InvalidCommandLineOptions) {
		"--idmap-path", GetIdmapPath(),
		"--policy", "this-does-not-exist"});
		// clang-format on
		ASSERT_THAT(result, NotNull());
		ASSERT_NE(result->status, EXIT_SUCCESS);
		ASSERT_TRUE((bool)result);
		ASSERT_NE(result.status, EXIT_SUCCESS);
		}

		} // namespace android::idmap2

cmds/idmap2/tests/TestHelpers.h

+1 −1

Original line number	Diff line number	Diff line
		@@ -192,7 +192,7 @@ const unsigned char kIdmapRawData[] = {
		// 0x100 string contents "test"
		0x74, 0x65, 0x73, 0x74};

		const unsigned int kIdmapRawDataLen = 0x104;
		constexpr unsigned int kIdmapRawDataLen = std::size(kIdmapRawData);
		const unsigned int kIdmapRawDataOffset = 0x54;
		const unsigned int kIdmapRawDataTargetCrc = 0x1234;
		const unsigned int kIdmapRawOverlayCrc = 0x5678;

core/jni/com_android_internal_content_om_OverlayConfig.cpp

+10 −10

Original line number	Diff line number	Diff line
		@@ -66,23 +66,23 @@ static jobjectArray createIdmap(JNIEnv* env, jclass /clazz/, jstring targetPat
		argv.emplace_back("--ignore-overlayable");
		}

		const auto result = ExecuteBinary(argv);
		auto result = ExecuteBinary(argv);
		if (!result) {
		LOG(ERROR) << "failed to execute idmap2";
		return nullptr;
		}

		if (result->status != 0) {
		LOG(ERROR) << "idmap2: " << result->stderr_str;
		if (result.status != 0) {
		LOG(ERROR) << "idmap2: " << result.stderr_str;
		return nullptr;
		}

		// Return the paths of the idmaps created or updated during the idmap invocation.
		std::vector<std::string> idmap_paths;
		std::istringstream input(result->stdout_str);
		std::istringstream input(std::move(result.stdout_str));
		std::string path;
		while (std::getline(input, path)) {
		idmap_paths.push_back(path);
		idmap_paths.push_back(std::move(path));
		}

		jobjectArray array = env->NewObjectArray(idmap_paths.size(), g_stringClass, nullptr);
		@@ -90,7 +90,7 @@ static jobjectArray createIdmap(JNIEnv* env, jclass /clazz/, jstring targetPat
		return nullptr;
		}
		for (size_t i = 0; i < idmap_paths.size(); i++) {
		const std::string path = idmap_paths[i];
		const std::string& path = idmap_paths[i];
		jstring java_string = env->NewStringUTF(path.c_str());
		if (env->ExceptionCheck()) {
		return nullptr;

libs/androidfw/PosixUtils.cpp

+27 −27

Original line number	Diff line number	Diff line
		@@ -17,7 +17,7 @@
		#ifdef _WIN32
		// nothing to see here
		#else
		#include <memory>
		#include <optional>
		#include <string>
		#include <vector>

		@@ -29,45 +29,42 @@

		#include "androidfw/PosixUtils.h"

		namespace {

		std::unique_ptr<std::string> ReadFile(int fd) {
		std::unique_ptr<std::string> str(new std::string());
		static std::optional<std::string> ReadFile(int fd) {
		std::string str;
		char buf[1024];
		ssize_t r;
		while ((r = read(fd, buf, sizeof(buf))) > 0) {
		str->append(buf, r);
		str.append(buf, r);
		}
		if (r != 0) {
		return nullptr;
		}
		return str;
		return std::nullopt;
		}

		return std::move(str);
		}

		namespace android {
		namespace util {

		std::unique_ptr<ProcResult> ExecuteBinary(const std::vector<std::string>& argv) {
		int stdout[2]; // stdout[0] read, stdout[1] write
		ProcResult ExecuteBinary(const std::vector<std::string>& argv) {
		int stdout[2]; // [0] read, [1] write
		if (pipe(stdout) != 0) {
		PLOG(ERROR) << "pipe";
		return nullptr;
		PLOG(ERROR) << "out pipe";
		return ProcResult{-1};
		}

		int stderr[2]; // stdout[0] read, stdout[1] write
		int stderr[2]; // [0] read, [1] write
		if (pipe(stderr) != 0) {
		PLOG(ERROR) << "pipe";
		PLOG(ERROR) << "err pipe";
		close(stdout[0]);
		close(stdout[1]);
		return nullptr;
		return ProcResult{-1};
		}

		auto gid = getgid();
		auto uid = getuid();

		char const argv0 = (char const)malloc(sizeof(char) (argv.size() + 1));
		// better keep no C++ objects going into the child here
		auto argv0 = (char const*)malloc(sizeof(char) * (argv.size() + 1));
		for (size_t i = 0; i < argv.size(); i++) {
		argv0[i] = argv[i].c_str();
		}
		@@ -76,8 +73,12 @@ std::unique_ptr<ProcResult> ExecuteBinary(const std::vector<std::string>& argv)
		switch (pid) {
		case -1: // error
		free(argv0);
		close(stdout[0]);
		close(stdout[1]);
		close(stderr[0]);
		close(stderr[1]);
		PLOG(ERROR) << "fork";
		return nullptr;
		return ProcResult{-1};
		case 0: // child
		if (setgid(gid) != 0) {
		PLOG(ERROR) << "setgid";
		@@ -109,17 +110,16 @@ std::unique_ptr<ProcResult> ExecuteBinary(const std::vector<std::string>& argv)
		if (!WIFEXITED(status)) {
		close(stdout[0]);
		close(stderr[0]);
		return nullptr;
		return ProcResult{-1};
		}
		std::unique_ptr<ProcResult> result(new ProcResult());
		result->status = status;
		const auto out = ReadFile(stdout[0]);
		result->stdout_str = out ? *out : "";
		ProcResult result(status);
		auto out = ReadFile(stdout[0]);
		result.stdout_str = out ? std::move(*out) : "";
		close(stdout[0]);
		const auto err = ReadFile(stderr[0]);
		result->stderr_str = err ? *err : "";
		auto err = ReadFile(stderr[0]);
		result.stderr_str = err ? std::move(*err) : "";
		close(stderr[0]);
		return result;
		return std::move(result);
		}
		}