summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--code_writer.cpp17
-rw-r--r--code_writer.h1
-rw-r--r--generate_cpp.cpp24
-rw-r--r--generate_cpp_unittest.cpp69
-rw-r--r--io_delegate.cpp9
-rw-r--r--io_delegate.h2
-rw-r--r--tests/fake_io_delegate.cpp29
-rw-r--r--tests/fake_io_delegate.h10
8 files changed, 150 insertions, 11 deletions
diff --git a/code_writer.cpp b/code_writer.cpp
index 7425c39..1c15b3a 100644
--- a/code_writer.cpp
+++ b/code_writer.cpp
@@ -32,6 +32,7 @@ namespace {
class StringCodeWriter : public CodeWriter {
public:
StringCodeWriter(std::string* output_buffer) : output_(output_buffer) {}
+ virtual ~StringCodeWriter() = default;
bool Write(const char* format, ...) override {
va_list ap;
@@ -41,6 +42,8 @@ class StringCodeWriter : public CodeWriter {
return true;
}
+ bool Close() override { return true; }
+
private:
std::string* output_;
}; // class StringCodeWriter
@@ -50,8 +53,8 @@ class FileCodeWriter : public CodeWriter {
FileCodeWriter(FILE* output_file, bool close_on_destruction)
: output_(output_file),
close_on_destruction_(close_on_destruction) {}
- ~FileCodeWriter() {
- if (close_on_destruction_) {
+ virtual ~FileCodeWriter() {
+ if (close_on_destruction_ && output_ != nullptr) {
fclose(output_);
}
}
@@ -62,10 +65,20 @@ class FileCodeWriter : public CodeWriter {
va_start(ap, format);
success = vfprintf(output_, format, ap) >= 0;
va_end(ap);
+ no_error_ = no_error_ && success;
return success;
}
+ bool Close() override {
+ if (output_ != nullptr) {
+ no_error_ = fclose(output_) == 0 && no_error_;
+ output_ = nullptr;
+ }
+ return no_error_;
+ }
+
private:
+ bool no_error_ = true;
FILE* output_;
bool close_on_destruction_;
}; // class StringCodeWriter
diff --git a/code_writer.h b/code_writer.h
index 0519d74..7fac542 100644
--- a/code_writer.h
+++ b/code_writer.h
@@ -32,6 +32,7 @@ class CodeWriter {
// Write a formatted string to this writer in the usual printf sense.
// Returns false on error.
virtual bool Write(const char* format, ...) = 0;
+ virtual bool Close() = 0;
virtual ~CodeWriter() = default;
}; // class CodeWriter
diff --git a/generate_cpp.cpp b/generate_cpp.cpp
index 0635ac0..b17587e 100644
--- a/generate_cpp.cpp
+++ b/generate_cpp.cpp
@@ -666,12 +666,17 @@ bool WriteHeader(const CppOptions& options,
return false;
}
- // TODO(wiley): b/25026025 error checking for file I/O
- header->Write(io_delegate.GetCodeWriter(
- options.OutputHeaderDir() + OS_PATH_SEPARATOR +
- HeaderFile(interface, header_type)).get());
+ const string header_path = options.OutputHeaderDir() + OS_PATH_SEPARATOR +
+ HeaderFile(interface, header_type);
+ unique_ptr<CodeWriter> code_writer(io_delegate.GetCodeWriter(header_path));
+ header->Write(code_writer.get());
- return true;
+ const bool success = code_writer->Close();
+ if (!success) {
+ io_delegate.RemovePath(header_path);
+ }
+
+ return success;
}
} // namespace internals
@@ -705,15 +710,18 @@ bool GenerateCpp(const CppOptions& options,
return false;
}
- // TODO(wiley): b/25026025 error checking for file I/O.
- // If it fails, we should remove all the partial results.
unique_ptr<CodeWriter> writer = io_delegate.GetCodeWriter(
options.OutputCppFilePath());
interface_src->Write(writer.get());
client_src->Write(writer.get());
server_src->Write(writer.get());
- return true;
+ const bool success = writer->Close();
+ if (!success) {
+ io_delegate.RemovePath(options.OutputCppFilePath());
+ }
+
+ return success;
}
} // namespace cpp
diff --git a/generate_cpp_unittest.cpp b/generate_cpp_unittest.cpp
index 54916cf..bb1f490 100644
--- a/generate_cpp_unittest.cpp
+++ b/generate_cpp_unittest.cpp
@@ -16,6 +16,7 @@
#include <string>
+#include <base/stringprintf.h>
#include <gtest/gtest.h>
#include "aidl.h"
@@ -23,11 +24,13 @@
#include "ast_cpp.h"
#include "code_writer.h"
#include "generate_cpp.h"
+#include "os.h"
#include "tests/fake_io_delegate.h"
#include "tests/test_util.h"
#include "type_cpp.h"
using android::aidl::test::FakeIoDelegate;
+using android::base::StringPrintf;
using std::string;
using std::unique_ptr;
@@ -593,6 +596,72 @@ TEST_F(ComplexTypeInterfaceASTTest, GeneratesInterfaceSource) {
Compare(doc.get(), kExpectedComplexTypeInterfaceSourceOutput);
}
+namespace test_io_handling {
+
+const char kInputPath[] = "a/IFoo.aidl";
+const char kOutputPath[] = "output.cpp";
+const char kHeaderDir[] = "headers";
+const char kInterfaceHeaderRelPath[] = "a/IFoo.h";
+
+} // namespace test_io_handling
+
+class IoErrorHandlingTest : public ASTTest {
+ public:
+ IoErrorHandlingTest ()
+ : ASTTest(test_io_handling::kInputPath,
+ "package a; interface IFoo {}"),
+ options_(GetOptions()) {}
+
+ const unique_ptr<CppOptions> options_;
+
+ private:
+ static unique_ptr<CppOptions> GetOptions() {
+ using namespace test_io_handling;
+
+ const int argc = 4;
+ const char* cmdline[argc] = {
+ "aidl-cpp", kInputPath, kHeaderDir, kOutputPath
+ };
+ return CppOptions::Parse(argc, cmdline);
+ }
+};
+
+TEST_F(IoErrorHandlingTest, GenerateCorrectlyAbsentErrors) {
+ // Confirm that this is working correctly without I/O problems.
+ const unique_ptr<AidlInterface> interface = Parse();
+ ASSERT_NE(interface, nullptr);
+ ASSERT_TRUE(GenerateCpp(*options_, types_, *interface, io_delegate_));
+}
+
+TEST_F(IoErrorHandlingTest, HandlesBadHeaderWrite) {
+ using namespace test_io_handling;
+ const unique_ptr<AidlInterface> interface = Parse();
+ ASSERT_NE(interface, nullptr);
+
+ // Simulate issues closing the interface header.
+ const string header_path =
+ StringPrintf("%s%c%s", kHeaderDir, OS_PATH_SEPARATOR,
+ kInterfaceHeaderRelPath);
+ io_delegate_.AddBrokenFilePath(header_path);
+ ASSERT_FALSE(GenerateCpp(*options_, types_, *interface, io_delegate_));
+ // We should never attempt to write the C++ file if we fail writing headers.
+ ASSERT_FALSE(io_delegate_.GetWrittenContents(kOutputPath, nullptr));
+ // We should remove partial results.
+ ASSERT_TRUE(io_delegate_.PathWasRemoved(header_path));
+}
+
+TEST_F(IoErrorHandlingTest, HandlesBadCppWrite) {
+ using test_io_handling::kOutputPath;
+ const unique_ptr<AidlInterface> interface = Parse();
+ ASSERT_NE(interface, nullptr);
+
+ // Simulate issues closing the cpp file.
+ io_delegate_.AddBrokenFilePath(kOutputPath);
+ ASSERT_FALSE(GenerateCpp(*options_, types_, *interface, io_delegate_));
+ // We should remove partial results.
+ ASSERT_TRUE(io_delegate_.PathWasRemoved(kOutputPath));
+}
+
} // namespace cpp
} // namespace aidl
} // namespace android
diff --git a/io_delegate.cpp b/io_delegate.cpp
index 85faf67..8061262 100644
--- a/io_delegate.cpp
+++ b/io_delegate.cpp
@@ -24,6 +24,7 @@
#include <direct.h>
#else
#include <sys/stat.h>
+#include <unistd.h>
#endif
#include <base/strings.h>
@@ -126,5 +127,13 @@ unique_ptr<CodeWriter> IoDelegate::GetCodeWriter(
return GetFileWriter(file_path);
}
+void IoDelegate::RemovePath(const std::string& file_path) const {
+#ifdef _WIN32
+ _unlink(file_path.c_str());
+#else
+ unlink(file_path.c_str());
+#endif
+}
+
} // namespace android
} // namespace aidl
diff --git a/io_delegate.h b/io_delegate.h
index 4a78fec..e1086ce 100644
--- a/io_delegate.h
+++ b/io_delegate.h
@@ -54,6 +54,8 @@ class IoDelegate {
virtual std::unique_ptr<CodeWriter> GetCodeWriter(
const std::string& file_path) const;
+ virtual void RemovePath(const std::string& file_path) const;
+
private:
DISALLOW_COPY_AND_ASSIGN(IoDelegate);
}; // class IoDelegate
diff --git a/tests/fake_io_delegate.cpp b/tests/fake_io_delegate.cpp
index 39bb90f..df8f578 100644
--- a/tests/fake_io_delegate.cpp
+++ b/tests/fake_io_delegate.cpp
@@ -34,6 +34,13 @@ namespace android {
namespace aidl {
namespace test {
+// Claims to always write successfully, but can't close the file.
+class BrokenCodeWriter : public CodeWriter {
+ bool Write(const char* /* format */, ...) override { return true; }
+ bool Close() override { return false; }
+ virtual ~BrokenCodeWriter() = default;
+}; // class BrokenCodeWriter
+
unique_ptr<string> FakeIoDelegate::GetFileContents(
const string& relative_filename,
const string& content_suffix) const {
@@ -73,10 +80,17 @@ bool FakeIoDelegate::CreatedNestedDirs(
std::unique_ptr<CodeWriter> FakeIoDelegate::GetCodeWriter(
const std::string& file_path) const {
+ if (broken_files_.count(file_path) > 0) {
+ return unique_ptr<CodeWriter>(new BrokenCodeWriter);
+ }
+ removed_files_.erase(file_path);
written_file_contents_[file_path] = "";
return GetStringWriter(&written_file_contents_[file_path]);
}
+void FakeIoDelegate::RemovePath(const std::string& file_path) const {
+ removed_files_.insert(file_path);
+}
void FakeIoDelegate::SetFileContents(const string& filename,
const string& contents) {
@@ -104,15 +118,28 @@ void FakeIoDelegate::AddCompoundParcelable(const string& canonical_name,
SetFileContents(rel_path.value(), contents);
}
+void FakeIoDelegate::AddBrokenFilePath(const std::string& path) {
+ broken_files_.insert(path);
+}
+
bool FakeIoDelegate::GetWrittenContents(const string& path, string* content) {
const auto it = written_file_contents_.find(path);
if (it == written_file_contents_.end()) {
return false;
}
- *content = it->second;
+ if (content) {
+ *content = it->second;
+ }
return true;
}
+bool FakeIoDelegate::PathWasRemoved(const std::string& path) {
+ if (removed_files_.count(path) > 0) {
+ return true;
+ }
+ return false;
+}
+
void FakeIoDelegate::AddStub(const string& canonical_name,
const char* format_str) {
string package, class_name;
diff --git a/tests/fake_io_delegate.h b/tests/fake_io_delegate.h
index 5ec6115..f90009a 100644
--- a/tests/fake_io_delegate.h
+++ b/tests/fake_io_delegate.h
@@ -21,6 +21,7 @@
#include <map>
#include <memory>
+#include <set>
#include <string>
#include <vector>
@@ -47,6 +48,7 @@ class FakeIoDelegate : public IoDelegate {
const std::vector<std::string>& nested_subdirs) const override;
std::unique_ptr<CodeWriter> GetCodeWriter(
const std::string& file_path) const override;
+ void RemovePath(const std::string& file_path) const override;
// Methods added to facilitate testing.
void SetFileContents(const std::string& filename,
@@ -55,10 +57,13 @@ class FakeIoDelegate : public IoDelegate {
void AddStubInterface(const std::string& canonical_name);
void AddCompoundParcelable(const std::string& canonical_name,
const std::vector<std::string>& subclasses);
+ void AddBrokenFilePath(const std::string& path);
// Returns true iff we've previously written to |path|.
// When we return true, we'll set *contents to the written string.
bool GetWrittenContents(const std::string& path, std::string* content);
+ bool PathWasRemoved(const std::string& path);
+
private:
void AddStub(const std::string& canonical_name, const char* format_str);
// Remove leading "./" from |path|.
@@ -70,6 +75,11 @@ class FakeIoDelegate : public IoDelegate {
// intentionally by storing the written strings.
mutable std::map<std::string, std::string> written_file_contents_;
+ // We normally just write to strings in |written_file_contents_| but for
+ // files in this list, we simulate I/O errors.
+ std::set<std::string> broken_files_;
+ mutable std::set<std::string> removed_files_;
+
DISALLOW_COPY_AND_ASSIGN(FakeIoDelegate);
}; // class FakeIoDelegate
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-16 08:02:52 +0000