summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--cmds/idmap2/libidmap2/Idmap.cpp3
-rw-r--r--cmds/idmap2/tests/Idmap2BinaryTests.cpp81
-rw-r--r--cmds/idmap2/tests/TestHelpers.h2
-rw-r--r--core/jni/com_android_internal_content_om_OverlayConfig.cpp20
-rw-r--r--libs/androidfw/PosixUtils.cpp54
-rw-r--r--libs/androidfw/include/androidfw/PosixUtils.h14
-rw-r--r--libs/androidfw/tests/PosixUtils_test.cpp18
-rw-r--r--tools/aapt2/cmd/Optimize.h2
8 files changed, 98 insertions, 96 deletions
diff --git a/cmds/idmap2/libidmap2/Idmap.cpp b/cmds/idmap2/libidmap2/Idmap.cpp
index 444f91d144a2..813dff1c141c 100644
--- a/cmds/idmap2/libidmap2/Idmap.cpp
+++ b/cmds/idmap2/libidmap2/Idmap.cpp
@@ -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;
diff --git a/cmds/idmap2/tests/Idmap2BinaryTests.cpp b/cmds/idmap2/tests/Idmap2BinaryTests.cpp
index e1b782972d3c..5a7fcd519cfd 100644
--- a/cmds/idmap2/tests/Idmap2BinaryTests.cpp
+++ b/cmds/idmap2/tests/Idmap2BinaryTests.cpp
@@ -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
diff --git a/cmds/idmap2/tests/TestHelpers.h b/cmds/idmap2/tests/TestHelpers.h
index 45740e2d5a9e..cdc0b8fbb87e 100644
--- a/cmds/idmap2/tests/TestHelpers.h
+++ b/cmds/idmap2/tests/TestHelpers.h
@@ -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;
diff --git a/core/jni/com_android_internal_content_om_OverlayConfig.cpp b/core/jni/com_android_internal_content_om_OverlayConfig.cpp
index b37269cc3c26..52a933ab9003 100644
--- a/core/jni/com_android_internal_content_om_OverlayConfig.cpp
+++ b/core/jni/com_android_internal_content_om_OverlayConfig.cpp
@@ -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,11 +90,11 @@ 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];
- jstring java_string = env->NewStringUTF(path.c_str());
- if (env->ExceptionCheck()) {
- return nullptr;
- }
+ const std::string& path = idmap_paths[i];
+ jstring java_string = env->NewStringUTF(path.c_str());
+ if (env->ExceptionCheck()) {
+ return nullptr;
+ }
env->SetObjectArrayElement(array, i, java_string);
env->DeleteLocalRef(java_string);
}
diff --git a/libs/androidfw/PosixUtils.cpp b/libs/androidfw/PosixUtils.cpp
index 026912883a73..8ddc57240129 100644
--- a/libs/androidfw/PosixUtils.cpp
+++ b/libs/androidfw/PosixUtils.cpp
@@ -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 std::nullopt;
}
- return str;
-}
-
+ 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);
}
}
diff --git a/libs/androidfw/include/androidfw/PosixUtils.h b/libs/androidfw/include/androidfw/PosixUtils.h
index bb2084740a44..c46e5e6b3fb5 100644
--- a/libs/androidfw/include/androidfw/PosixUtils.h
+++ b/libs/androidfw/include/androidfw/PosixUtils.h
@@ -25,12 +25,18 @@ struct ProcResult {
int status;
std::string stdout_str;
std::string stderr_str;
+
+ explicit ProcResult(int status) : status(status) {}
+ ProcResult(ProcResult&&) noexcept = default;
+ ProcResult& operator=(ProcResult&&) noexcept = default;
+
+ explicit operator bool() const { return status >= 0; }
};
-// Fork, exec and wait for an external process. Return nullptr if the process could not be launched,
-// otherwise a ProcResult containing the external process' exit status and captured stdout and
-// stderr.
-std::unique_ptr<ProcResult> ExecuteBinary(const std::vector<std::string>& argv);
+// Fork, exec and wait for an external process. Returns status < 0 if the process could not be
+// launched, otherwise a ProcResult containing the external process' exit status and captured
+// stdout and stderr.
+ProcResult ExecuteBinary(const std::vector<std::string>& argv);
} // namespace util
} // namespace android
diff --git a/libs/androidfw/tests/PosixUtils_test.cpp b/libs/androidfw/tests/PosixUtils_test.cpp
index 8c49350796ec..097e6b0bba65 100644
--- a/libs/androidfw/tests/PosixUtils_test.cpp
+++ b/libs/androidfw/tests/PosixUtils_test.cpp
@@ -28,27 +28,27 @@ namespace util {
TEST(PosixUtilsTest, AbsolutePathToBinary) {
const auto result = ExecuteBinary({"/bin/date", "--help"});
- ASSERT_THAT(result, NotNull());
- ASSERT_EQ(result->status, 0);
- ASSERT_GE(result->stdout_str.find("usage: date "), 0);
+ ASSERT_TRUE((bool)result);
+ ASSERT_EQ(result.status, 0);
+ ASSERT_GE(result.stdout_str.find("usage: date "), 0);
}
TEST(PosixUtilsTest, RelativePathToBinary) {
const auto result = ExecuteBinary({"date", "--help"});
- ASSERT_THAT(result, NotNull());
- ASSERT_EQ(result->status, 0);
- ASSERT_GE(result->stdout_str.find("usage: date "), 0);
+ ASSERT_TRUE((bool)result);
+ ASSERT_EQ(result.status, 0);
+ ASSERT_GE(result.stdout_str.find("usage: date "), 0);
}
TEST(PosixUtilsTest, BadParameters) {
const auto result = ExecuteBinary({"/bin/date", "--this-parameter-is-not-supported"});
- ASSERT_THAT(result, NotNull());
- ASSERT_NE(result->status, 0);
+ ASSERT_TRUE((bool)result);
+ ASSERT_GT(result.status, 0);
}
TEST(PosixUtilsTest, NoSuchBinary) {
const auto result = ExecuteBinary({"/this/binary/does/not/exist"});
- ASSERT_THAT(result, IsNull());
+ ASSERT_FALSE((bool)result);
}
} // android
diff --git a/tools/aapt2/cmd/Optimize.h b/tools/aapt2/cmd/Optimize.h
index 10b84b0c6bda..790bb74b2566 100644
--- a/tools/aapt2/cmd/Optimize.h
+++ b/tools/aapt2/cmd/Optimize.h
@@ -26,8 +26,6 @@
namespace aapt {
struct OptimizeOptions {
- friend class OptimizeCommand;
-
// Path to the output APK.
std::optional<std::string> output_path;
// Path to the output APK directory for splits.
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-10-21 07:54:24 +0000