Skip to content

Commit

Permalink
Fix "readability-redundant-string-cstr" clang-tidy issues
Browse files Browse the repository at this point in the history
Bug: 264654008
Test: m tidy-art
Change-Id: Ic852d58f106187791fa3a0d31829654de41bcb2b
  • Loading branch information
Stefano Cianciulli committed Apr 5, 2023
1 parent 2bb753e commit ba87ab5
Show file tree
Hide file tree
Showing 31 changed files with 973 additions and 1,142 deletions.
1 change: 1 addition & 0 deletions CPPLINT.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ filter=-readability/function,-readability/streams,-readability/todo
filter=-runtime/printf,-runtime/references,-runtime/sizeof,-runtime/threadsafe_fn
# TODO: this should be re-enabled.
filter=-whitespace/line_length
filter=-whitespace/braces
1 change: 1 addition & 0 deletions build/Android.bp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ art_clang_tidy_errors = [
"performance-noexcept-move-constructor",
"performance-unnecessary-copy-initialization",
"performance-unnecessary-value-param",
"readability-redundant-string-cstr",
]

art_clang_tidy_disabled = [
Expand Down
2 changes: 1 addition & 1 deletion compiler/utils/assembler_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class AssemblerTestBase : public testing::Test {

// Assemble reference object file.
std::string ref_obj_file = test_path(".ref.o");
ASSERT_TRUE(Assemble(ref_asm_file.c_str(), ref_obj_file.c_str()));
ASSERT_TRUE(Assemble(ref_asm_file, ref_obj_file));

// Read the code produced by assembler from the ELF file.
std::vector<uint8_t> ref_code;
Expand Down
2 changes: 1 addition & 1 deletion dex2oat/dex2oat.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1906,7 +1906,7 @@ class Dex2Oat final {
}
}

if (android::base::StartsWith(dex_location, filter.c_str())) {
if (android::base::StartsWith(dex_location, filter)) {
VLOG(compiler) << "Disabling inlining from " << dex_file->GetLocation();
no_inline_from_dex_files.push_back(dex_file);
break;
Expand Down
951 changes: 442 additions & 509 deletions dex2oat/dex2oat_test.cc

Large diffs are not rendered by default.

67 changes: 32 additions & 35 deletions dex2oat/dex2oat_vdex_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class Dex2oatVdexTest : public Dex2oatEnvironmentTest {
args.push_back("--public-sdk=" + *public_sdk);
}
args.push_back("--compiler-filter=" +
CompilerFilter::NameOfFilter(CompilerFilter::Filter::kVerify));
CompilerFilter::NameOfFilter(CompilerFilter::Filter::kVerify));
args.push_back("--runtime-arg");
args.push_back("-Xnorelocate");
if (!copy_dex_files) {
Expand All @@ -65,12 +65,12 @@ class Dex2oatVdexTest : public Dex2oatEnvironmentTest {
return Dex2Oat(args, &output_, &error_msg_) == 0;
}

std::unique_ptr<VerifierDeps> GetVerifierDeps(
const std::string& vdex_location, const DexFile* dex_file) {
std::unique_ptr<VerifierDeps> GetVerifierDeps(const std::string& vdex_location,
const DexFile* dex_file) {
// Verify the vdex file content: only the classes using public APIs should be verified.
std::unique_ptr<VdexFile> vdex(VdexFile::Open(vdex_location.c_str(),
/*writable=*/ false,
/*low_4gb=*/ false,
std::unique_ptr<VdexFile> vdex(VdexFile::Open(vdex_location,
/*writable=*/false,
/*low_4gb=*/false,
&error_msg_));
// Check the vdex doesn't have dex.
if (vdex->HasDexSection()) {
Expand All @@ -85,7 +85,7 @@ class Dex2oatVdexTest : public Dex2oatEnvironmentTest {

std::vector<const DexFile*> dex_files;
dex_files.push_back(dex_file);
std::unique_ptr<VerifierDeps> deps(new VerifierDeps(dex_files, /*output_only=*/ false));
std::unique_ptr<VerifierDeps> deps(new VerifierDeps(dex_files, /*output_only=*/false));

if (!deps->ParseStoredData(dex_files, vdex->GetVerifierDepsData())) {
::testing::AssertionFailure() << error_msg_;
Expand Down Expand Up @@ -200,11 +200,10 @@ TEST_F(Dex2oatVdexTest, VerifyPublicSdkStubsWithDexFiles) {
std::unique_ptr<const DexFile> dex_file(OpenTestDexFile("Dex2oatVdexTestDex"));

// Compile the subject app using the predefined API-stubs
ASSERT_TRUE(RunDex2oat(
dex_file->GetLocation(),
GetOdex(dex_file),
/*public_sdk=*/ nullptr,
/*copy_dex_files=*/ true));
ASSERT_TRUE(RunDex2oat(dex_file->GetLocation(),
GetOdex(dex_file),
/*public_sdk=*/nullptr,
/*copy_dex_files=*/true));

// Create the .dm file with the output.
std::string dm_file = GetScratchDir() + "/base.dm";
Expand All @@ -214,12 +213,11 @@ TEST_F(Dex2oatVdexTest, VerifyPublicSdkStubsWithDexFiles) {

// Recompile again with the .dm file which contains a vdex with code.
// The compilation will pass, but dex2oat will not use the vdex file.
ASSERT_TRUE(RunDex2oat(
dex_file->GetLocation(),
GetOdex(dex_file, "v2"),
/*public_sdk=*/ nullptr,
/*copy_dex_files=*/ true,
extra_args));
ASSERT_TRUE(RunDex2oat(dex_file->GetLocation(),
GetOdex(dex_file, "v2"),
/*public_sdk=*/nullptr,
/*copy_dex_files=*/true,
extra_args));
}

// Check that corrupt vdex files from .dm archives are ignored.
Expand All @@ -238,12 +236,12 @@ TEST_F(Dex2oatVdexTest, VerifyCorruptVdexFile) {
extra_args.push_back("--dm-file=" + dm_file);

// Compile the dex file. Despite having a corrupt input .vdex, we should not crash.
ASSERT_TRUE(RunDex2oat(
dex_file->GetLocation(),
GetOdex(dex_file),
/*public_sdk=*/ nullptr,
/*copy_dex_files=*/ true,
extra_args)) << output_;
ASSERT_TRUE(RunDex2oat(dex_file->GetLocation(),
GetOdex(dex_file),
/*public_sdk=*/nullptr,
/*copy_dex_files=*/true,
extra_args))
<< output_;
}

// Check that if the input dm a vdex with mismatching checksums the compilation fails
Expand All @@ -253,11 +251,10 @@ TEST_F(Dex2oatVdexTest, VerifyInputDmWithMismatchedChecksums) {
// Generate a vdex file for Dex2oatVdexTestDex.
std::unique_ptr<const DexFile> dex_file(OpenTestDexFile("Dex2oatVdexTestDex"));

ASSERT_TRUE(RunDex2oat(
dex_file->GetLocation(),
GetOdex(dex_file),
/*public_sdk=*/ nullptr,
/*copy_dex_files=*/ false));
ASSERT_TRUE(RunDex2oat(dex_file->GetLocation(),
GetOdex(dex_file),
/*public_sdk=*/nullptr,
/*copy_dex_files=*/false));

// Create the .dm file with the output.
std::string dm_file = GetScratchDir() + "/base.dm";
Expand All @@ -268,12 +265,12 @@ TEST_F(Dex2oatVdexTest, VerifyInputDmWithMismatchedChecksums) {
// Try to compile Main using an input dm which contains the vdex for
// Dex2oatVdexTestDex. It should fail.
std::unique_ptr<const DexFile> dex_file2(OpenTestDexFile("Main"));
ASSERT_FALSE(RunDex2oat(
dex_file2->GetLocation(),
GetOdex(dex_file2, "v2"),
/*public_sdk=*/ nullptr,
/*copy_dex_files=*/ false,
extra_args)) << output_;
ASSERT_FALSE(RunDex2oat(dex_file2->GetLocation(),
GetOdex(dex_file2, "v2"),
/*public_sdk=*/nullptr,
/*copy_dex_files=*/false,
extra_args))
<< output_;
}

} // namespace art
41 changes: 19 additions & 22 deletions dexlayout/dexdiag_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ static const char* kDexDiagBinaryName = "dexdiag";

class DexDiagTest : public CommonArtTest {
protected:
void SetUp() override {
CommonArtTest::SetUp();
}
void SetUp() override { CommonArtTest::SetUp(); }

// Path to the dexdiag(d?)[32|64] binary.
std::string GetDexDiagFilePath() {
Expand Down Expand Up @@ -63,11 +61,11 @@ class DexDiagTest : public CommonArtTest {
EXPECT_TRUE(!oat_location.empty());
std::cout << "==" << oat_location << std::endl;
std::string error_msg;
std::unique_ptr<OatFile> oat(OatFile::Open(/*zip_fd=*/ -1,
oat_location.c_str(),
oat_location.c_str(),
/*executable=*/ false,
/*low_4gb=*/ false,
std::unique_ptr<OatFile> oat(OatFile::Open(/*zip_fd=*/-1,
oat_location,
oat_location,
/*executable=*/false,
/*low_4gb=*/false,
&error_msg));
EXPECT_TRUE(oat != nullptr) << error_msg;
return oat;
Expand All @@ -82,8 +80,8 @@ class DexDiagTest : public CommonArtTest {

// Build the command line "dexdiag <args> this_pid".
std::string executable_path = GetDexDiagFilePath();
EXPECT_TRUE(OS::FileExists(executable_path.c_str())) << executable_path
<< " should be a valid file path";
EXPECT_TRUE(OS::FileExists(executable_path.c_str()))
<< executable_path << " should be a valid file path";
exec_argv.push_back(executable_path);
for (const auto& arg : args) {
exec_argv.push_back(arg);
Expand All @@ -102,45 +100,44 @@ class DexDiagTest : public CommonArtTest {
TEST_F(DexDiagTest, DexDiagHelpTest) {
// TODO: test the resulting output.
std::string error_msg;
ASSERT_TRUE(Exec(getpid(), { kDexDiagHelp }, &error_msg)) << "Failed to execute -- because: "
<< error_msg;
ASSERT_TRUE(Exec(getpid(), {kDexDiagHelp}, &error_msg))
<< "Failed to execute -- because: " << error_msg;
}

#if defined (ART_TARGET)
#if defined(ART_TARGET)
TEST_F(DexDiagTest, DexDiagContainsTest) {
#else
TEST_F(DexDiagTest, DISABLED_DexDiagContainsTest) {
#endif
std::unique_ptr<OatFile> oat = OpenOatAndVdexFiles();
// TODO: test the resulting output.
std::string error_msg;
ASSERT_TRUE(Exec(getpid(), { kDexDiagContains }, &error_msg)) << "Failed to execute -- because: "
<< error_msg;
ASSERT_TRUE(Exec(getpid(), {kDexDiagContains}, &error_msg))
<< "Failed to execute -- because: " << error_msg;
}

#if defined (ART_TARGET)
#if defined(ART_TARGET)
TEST_F(DexDiagTest, DexDiagContainsFailsTest) {
#else
TEST_F(DexDiagTest, DISABLED_DexDiagContainsFailsTest) {
#endif
std::unique_ptr<OatFile> oat = OpenOatAndVdexFiles();
// TODO: test the resulting output.
std::string error_msg;
ASSERT_FALSE(Exec(getpid(), { kDexDiagContainsFails }, &error_msg))
<< "Failed to execute -- because: "
<< error_msg;
ASSERT_FALSE(Exec(getpid(), {kDexDiagContainsFails}, &error_msg))
<< "Failed to execute -- because: " << error_msg;
}

#if defined (ART_TARGET)
#if defined(ART_TARGET)
TEST_F(DexDiagTest, DexDiagVerboseTest) {
#else
TEST_F(DexDiagTest, DISABLED_DexDiagVerboseTest) {
#endif
// TODO: test the resulting output.
std::unique_ptr<OatFile> oat = OpenOatAndVdexFiles();
std::string error_msg;
ASSERT_TRUE(Exec(getpid(), { kDexDiagVerbose }, &error_msg)) << "Failed to execute -- because: "
<< error_msg;
ASSERT_TRUE(Exec(getpid(), {kDexDiagVerbose}, &error_msg))
<< "Failed to execute -- because: " << error_msg;
}

} // namespace art
18 changes: 9 additions & 9 deletions dexoptanalyzer/dexoptanalyzer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ TEST_F(DexoptAnalyzerTest, OatUpToDate) {
std::string dex_location = GetScratchDir() + "/OatUpToDate.jar";
std::string odex_location = GetOdexDir() + "/OatUpToDate.odex";
Copy(GetDexSrc1(), dex_location);
GenerateOdexForTest(dex_location.c_str(), odex_location.c_str(), CompilerFilter::kSpeed);
GenerateOdexForTest(dex_location, odex_location, CompilerFilter::kSpeed);

Verify(dex_location, CompilerFilter::kSpeed);
Verify(dex_location, CompilerFilter::kVerify);
Expand All @@ -143,7 +143,7 @@ TEST_F(DexoptAnalyzerTest, ProfileOatUpToDate) {
std::string dex_location = GetScratchDir() + "/ProfileOatUpToDate.jar";
std::string odex_location = GetOdexDir() + "/ProfileOatUpToDate.odex";
Copy(GetDexSrc1(), dex_location);
GenerateOdexForTest(dex_location.c_str(), odex_location.c_str(), CompilerFilter::kSpeedProfile);
GenerateOdexForTest(dex_location, odex_location, CompilerFilter::kSpeedProfile);

Verify(dex_location, CompilerFilter::kSpeedProfile,
ProfileAnalysisResult::kDontOptimizeSmallDelta);
Expand All @@ -159,7 +159,7 @@ TEST_F(DexoptAnalyzerTest, VerifyAndEmptyProfiles) {
std::string odex_location = GetOdexDir() + "/VerifyAndEmptyProfiles.odex";
Copy(GetDexSrc1(), dex_location);

GenerateOdexForTest(dex_location.c_str(), odex_location.c_str(), CompilerFilter::kVerify);
GenerateOdexForTest(dex_location, odex_location, CompilerFilter::kVerify);

// If we want to speed-profile something that was verified, do it even if
// the profile analysis returns kDontOptimizeSmallDelta (it means that we do have profile data,
Expand All @@ -186,7 +186,7 @@ TEST_F(DexoptAnalyzerTest, Downgrade) {
std::string dex_location = GetScratchDir() + "/Downgrade.jar";
std::string odex_location = GetOdexDir() + "/Downgrade.odex";
Copy(GetDexSrc1(), dex_location);
GenerateOdexForTest(dex_location.c_str(), odex_location.c_str(), CompilerFilter::kVerify);
GenerateOdexForTest(dex_location, odex_location, CompilerFilter::kVerify);

Verify(dex_location, CompilerFilter::kSpeedProfile,
ProfileAnalysisResult::kDontOptimizeSmallDelta, true);
Expand All @@ -200,7 +200,7 @@ TEST_F(DexoptAnalyzerTest, MultiDexOatUpToDate) {
std::string odex_location = GetOdexDir() + "/MultiDexOatUpToDate.odex";

Copy(GetMultiDexSrc1(), dex_location);
GenerateOdexForTest(dex_location.c_str(), odex_location.c_str(), CompilerFilter::kSpeed);
GenerateOdexForTest(dex_location, odex_location, CompilerFilter::kSpeed);

Verify(dex_location, CompilerFilter::kSpeed, ProfileAnalysisResult::kDontOptimizeSmallDelta);
}
Expand All @@ -212,7 +212,7 @@ TEST_F(DexoptAnalyzerTest, MultiDexSecondaryOutOfDate) {

// Compile code for GetMultiDexSrc1.
Copy(GetMultiDexSrc1(), dex_location);
GenerateOdexForTest(dex_location.c_str(), odex_location.c_str(), CompilerFilter::kSpeed);
GenerateOdexForTest(dex_location, odex_location, CompilerFilter::kSpeed);

// Now overwrite the dex file with GetMultiDexSrc2 so the secondary checksum
// is out of date.
Expand All @@ -230,7 +230,7 @@ TEST_F(DexoptAnalyzerTest, OatDexOutOfDate) {
// We create a dex, generate an oat for it, then overwrite the dex with a
// different dex to make the oat out of date.
Copy(GetDexSrc1(), dex_location);
GenerateOdexForTest(dex_location.c_str(), odex_location.c_str(), CompilerFilter::kSpeed);
GenerateOdexForTest(dex_location, odex_location, CompilerFilter::kSpeed);
Copy(GetDexSrc2(), dex_location);

Verify(dex_location, CompilerFilter::kSpeed);
Expand All @@ -243,8 +243,8 @@ TEST_F(DexoptAnalyzerTest, OatImageOutOfDate) {
std::string odex_location = GetOdexDir() + "/OatImageOutOfDate.odex";

Copy(GetDexSrc1(), dex_location);
GenerateOatForTest(dex_location.c_str(),
odex_location.c_str(),
GenerateOatForTest(dex_location,
odex_location,
CompilerFilter::kSpeed,
/*with_alternate_image=*/true);

Expand Down
10 changes: 5 additions & 5 deletions libartbase/base/common_art_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ void CommonArtTestImpl::SetUpAndroidRootEnvVars() {
const char* android_i18n_root_from_env = getenv("ANDROID_I18N_ROOT");
if (android_i18n_root_from_env == nullptr) {
// Use ${ANDROID_I18N_OUT}/com.android.i18n for ANDROID_I18N_ROOT.
std::string android_i18n_root = android_host_out.c_str();
std::string android_i18n_root = android_host_out;
android_i18n_root += "/com.android.i18n";
setenv("ANDROID_I18N_ROOT", android_i18n_root.c_str(), 1);
}
Expand All @@ -277,7 +277,7 @@ void CommonArtTestImpl::SetUpAndroidRootEnvVars() {
const char* android_art_root_from_env = getenv("ANDROID_ART_ROOT");
if (android_art_root_from_env == nullptr) {
// Use ${ANDROID_HOST_OUT}/com.android.art for ANDROID_ART_ROOT.
std::string android_art_root = android_host_out.c_str();
std::string android_art_root = android_host_out;
android_art_root += "/com.android.art";
setenv("ANDROID_ART_ROOT", android_art_root.c_str(), 1);
}
Expand All @@ -288,7 +288,7 @@ void CommonArtTestImpl::SetUpAndroidRootEnvVars() {
const char* android_tzdata_root_from_env = getenv("ANDROID_TZDATA_ROOT");
if (android_tzdata_root_from_env == nullptr) {
// Use ${ANDROID_HOST_OUT}/com.android.tzdata for ANDROID_TZDATA_ROOT.
std::string android_tzdata_root = android_host_out.c_str();
std::string android_tzdata_root = android_host_out;
android_tzdata_root += "/com.android.tzdata";
setenv("ANDROID_TZDATA_ROOT", android_tzdata_root.c_str(), 1);
}
Expand Down Expand Up @@ -328,7 +328,7 @@ void CommonArtTestImpl::SetUp() {
SetUpAndroidDataDir(android_data_);

// Re-use the data temporary directory for /system_ext tests
android_system_ext_.append(android_data_.c_str());
android_system_ext_.append(android_data_);
android_system_ext_.append("/system_ext");
int mkdir_result = mkdir(android_system_ext_.c_str(), 0700);
ASSERT_EQ(mkdir_result, 0);
Expand All @@ -338,7 +338,7 @@ void CommonArtTestImpl::SetUp() {
mkdir_result = mkdir(system_ext_framework.c_str(), 0700);
ASSERT_EQ(mkdir_result, 0);

dalvik_cache_.append(android_data_.c_str());
dalvik_cache_.append(android_data_);
dalvik_cache_.append("/dalvik-cache");
mkdir_result = mkdir(dalvik_cache_.c_str(), 0700);
ASSERT_EQ(mkdir_result, 0);
Expand Down
2 changes: 1 addition & 1 deletion libartbase/base/file_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ bool LocationIsOnSystem(const std::string& location) {
LOG(FATAL) << "LocationIsOnSystem is unsupported on Windows.";
return false;
#else
return android::base::StartsWith(location, GetAndroidRoot().c_str());
return android::base::StartsWith(location, GetAndroidRoot());
#endif
}

Expand Down
Loading

0 comments on commit ba87ab5

Please sign in to comment.