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
diff --git a/Android.mk b/Android.mk
index 5b844e4..b27a4df 100644
--- a/Android.mk
+++ b/Android.mk
@@ -99,6 +99,7 @@
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 6f542bf..10a90e4 100644
--- a/aidl.cpp
+++ b/aidl.cpp
@@ -77,23 +77,10 @@
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 66891dd..62be3f5 100644
--- a/io_delegate.cpp
+++ b/io_delegate.cpp
@@ -41,6 +41,45 @@
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<string> IoDelegate::GetFileContents(
const string& filename,
const string& content_suffix) const {
@@ -112,15 +151,26 @@
return true;
}
- string base = ".";
- if (path[0] == OS_PATH_SEPARATOR) {
- base = "/";
+ string absolute_path;
+ if (!GetAbsolutePath(path, &absolute_path)) {
+ return false;
}
- auto split = Split(path, string{1u, OS_PATH_SEPARATOR});
- split.pop_back();
+ auto directories = Split(absolute_path, string{1u, OS_PATH_SEPARATOR});
- return CreatedNestedDirs(base, split);
+ // 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());
+
+ // Remove the actual file in question, we're just creating the directory path.
+ directories.pop_back();
+
+ return CreatedNestedDirs(base, directories);
}
unique_ptr<CodeWriter> IoDelegate::GetCodeWriter(
diff --git a/io_delegate.h b/io_delegate.h
index b0a703b..dc9a3e3 100644
--- a/io_delegate.h
+++ b/io_delegate.h
@@ -34,6 +34,12 @@
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<std::string> GetFileContents(
diff --git a/io_delegate_unittest.cpp b/io_delegate_unittest.cpp
new file mode 100644
index 0000000..5227d0b
--- /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 <string>
+
+#include <gtest/gtest.h>
+
+#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
