From 8decf95ffeffcc8aa14aa743ae398bc38d54b0ac Mon Sep 17 00:00:00 2001 From: Christopher Wiley Date: Thu, 2 Jun 2016 16:27:26 -0700 Subject: Always build absolute paths the same way Fix a bug where output paths like: C:\foo\bar triggered bad logic to detect relative paths and turned the path into .\C:\foo\bar Fix this by moving absolute path detection logic into a common helper function and calling it consistently. Bug: 29091703 Test: unittests, angler-eng builds with this change, manual testing on windows demonstrates issue is fixed Change-Id: I8bc83a3e3943931d95db3e4147d4f8b1b07e907a --- Android.mk | 1 + aidl.cpp | 17 ++------------ io_delegate.cpp | 60 ++++++++++++++++++++++++++++++++++++++++++++---- io_delegate.h | 6 +++++ io_delegate_unittest.cpp | 46 +++++++++++++++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 20 deletions(-) create mode 100644 io_delegate_unittest.cpp diff --git a/Android.mk b/Android.mk index 5b844e4e..b27a4dfb 100644 --- a/Android.mk +++ b/Android.mk @@ -99,6 +99,7 @@ LOCAL_SRC_FILES := \ ast_cpp_unittest.cpp \ ast_java_unittest.cpp \ generate_cpp_unittest.cpp \ + io_delegate_unittest.cpp \ options_unittest.cpp \ tests/end_to_end_tests.cpp \ tests/fake_io_delegate.cpp \ diff --git a/aidl.cpp b/aidl.cpp index 6f542bfb..10a90e46 100644 --- a/aidl.cpp +++ b/aidl.cpp @@ -77,23 +77,10 @@ bool check_filename(const std::string& filename, string expected; string fn; size_t len; - char cwd[MAXPATHLEN]; bool valid = false; -#ifdef _WIN32 - if (isalpha(filename[0]) && filename[1] == ':' - && filename[2] == OS_PATH_SEPARATOR) { -#else - if (filename[0] == OS_PATH_SEPARATOR) { -#endif - fn = filename; - } else { - fn = getcwd(cwd, sizeof(cwd)); - len = fn.length(); - if (fn[len-1] != OS_PATH_SEPARATOR) { - fn += OS_PATH_SEPARATOR; - } - fn += filename; + if (!IoDelegate::GetAbsolutePath(filename, &fn)) { + return false; } if (!package.empty()) { diff --git a/io_delegate.cpp b/io_delegate.cpp index 66891dd1..62be3f5c 100644 --- a/io_delegate.cpp +++ b/io_delegate.cpp @@ -41,6 +41,45 @@ using android::base::Split; namespace android { namespace aidl { +bool IoDelegate::GetAbsolutePath(const string& path, string* absolute_path) { +#ifdef _WIN32 + + char buf[4096]; + DWORD path_len = GetFullPathName(path.c_str(), sizeof(buf), buf, nullptr); + if (path_len <= 0 || path_len >= sizeof(buf)) { + LOG(ERROR) << "Failed to GetFullPathName(" << path << ")"; + return false; + } + *absolute_path = buf; + + return true; + +#else + + if (path.empty()) { + LOG(ERROR) << "Giving up on finding an absolute path to represent the " + "empty string."; + return false; + } + if (path[0] == OS_PATH_SEPARATOR) { + *absolute_path = path; + return true; + } + + char buf[4096]; + if (getcwd(buf, sizeof(buf)) == nullptr) { + LOG(ERROR) << "Path of current working directory does not fit in " + << sizeof(buf) << " bytes"; + return false; + } + + *absolute_path = buf; + *absolute_path += OS_PATH_SEPARATOR; + *absolute_path += path; + return true; +#endif +} + unique_ptr IoDelegate::GetFileContents( const string& filename, const string& content_suffix) const { @@ -112,15 +151,26 @@ bool IoDelegate::CreatePathForFile(const string& path) const { return true; } - string base = "."; - if (path[0] == OS_PATH_SEPARATOR) { + string absolute_path; + if (!GetAbsolutePath(path, &absolute_path)) { + return false; + } + + auto directories = Split(absolute_path, string{1u, OS_PATH_SEPARATOR}); + + // The "base" directory is just the root of the file system. On Windows, + // this will look like "C:\" but on Unix style file systems we get an empty + // string after splitting "/foo" with "/" + string base = directories[0]; + if (base.empty()) { base = "/"; } + directories.erase(directories.begin()); - auto split = Split(path, string{1u, OS_PATH_SEPARATOR}); - split.pop_back(); + // Remove the actual file in question, we're just creating the directory path. + directories.pop_back(); - return CreatedNestedDirs(base, split); + return CreatedNestedDirs(base, directories); } unique_ptr IoDelegate::GetCodeWriter( diff --git a/io_delegate.h b/io_delegate.h index b0a703b3..dc9a3e3e 100644 --- a/io_delegate.h +++ b/io_delegate.h @@ -34,6 +34,12 @@ class IoDelegate { IoDelegate() = default; virtual ~IoDelegate() = default; + // Stores an absolute version of |path| to |*absolute_path|, + // possibly prefixing it with the current working directory. + // Returns false and does not set |*absolute_path| on error. + static bool GetAbsolutePath(const std::string& path, + std::string* absolute_path); + // Returns a unique_ptr to the contents of |filename|. // Will append the optional |content_suffix| to the returned contents. virtual std::unique_ptr GetFileContents( diff --git a/io_delegate_unittest.cpp b/io_delegate_unittest.cpp new file mode 100644 index 00000000..5227d0bf --- /dev/null +++ b/io_delegate_unittest.cpp @@ -0,0 +1,46 @@ +/* + * Copyright (C) 2016, The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include "io_delegate.h" + +using std::string; + +namespace android { +namespace aidl { + +TEST(IoDelegateTest, CannotGetAbsolutePathFromEmptyString) { + string absolute_path; + EXPECT_FALSE(IoDelegate::GetAbsolutePath("", &absolute_path)); + EXPECT_TRUE(absolute_path.empty()); +} + +TEST(IoDelegateTest, CurrentlyInfersLinuxAbsolutePath) { + string absolute_path; + EXPECT_TRUE(IoDelegate::GetAbsolutePath("foo", &absolute_path)); + ASSERT_FALSE(absolute_path.empty()); + // Should find our desired file at the end of |absolute_path| + // But we don't know the prefix, since it's the current working directory + EXPECT_TRUE(absolute_path.rfind("/foo") == absolute_path.length() - 4); + // Whatever our current working directory, the path is absolute. + EXPECT_EQ(absolute_path[0], '/'); +} + +} // namespace android +} // namespace aidl -- cgit v1.2.3