lawrence_danna updated this revision to Diff 221985.
lawrence_danna marked 3 inline comments as done.
lawrence_danna added a comment.
fixed the remaining comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67996/new/
https://reviews.llvm.org/D67996
Files:
lldb/include/lldb/Core/StreamFile.h
lldb/include/lldb/Host/FileCache.h
lldb/include/lldb/Host/FileSystem.h
lldb/include/lldb/lldb-forward.h
lldb/scripts/Python/python-typemaps.swig
lldb/source/API/SBStream.cpp
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Core/StreamFile.cpp
lldb/source/Expression/REPL.cpp
lldb/source/Host/common/FileCache.cpp
lldb/source/Host/common/FileSystem.cpp
lldb/source/Host/windows/Host.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Target/ModuleCache.cpp
lldb/source/Target/Platform.cpp
lldb/unittests/Host/FileSystemTest.cpp
lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===================================================================
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -15,6 +15,7 @@
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/lldb-enumerations.h"
+#include "llvm/Testing/Support/Error.h"
#include "PythonTestSuite.h"
@@ -581,10 +582,10 @@
}
TEST_F(PythonDataObjectsTest, TestPythonFile) {
- File file;
- FileSystem::Instance().Open(file, FileSpec(FileSystem::DEV_NULL),
- File::eOpenOptionRead);
- PythonFile py_file(file, "r");
+ auto file = FileSystem::Instance().Open(FileSpec(FileSystem::DEV_NULL),
+ File::eOpenOptionRead);
+ ASSERT_THAT_EXPECTED(file, llvm::Succeeded());
+ PythonFile py_file(*file.get(), "r");
EXPECT_TRUE(PythonFile::Check(py_file.get()));
}
Index: lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt
===================================================================
--- lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt
+++ lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt
@@ -6,6 +6,7 @@
LINK_LIBS
lldbHost
lldbPluginScriptInterpreterPython
+ LLVMTestingSupport
LINK_COMPONENTS
Support
)
\ No newline at end of file
Index: lldb/unittests/Host/FileSystemTest.cpp
===================================================================
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -288,3 +288,18 @@
EXPECT_THAT(visited,
testing::UnorderedElementsAre("/foo", "/bar", "/baz", "/qux"));
}
+
+TEST(FileSystemTest, OpenErrno) {
+#ifdef _WIN32
+ FileSpec spec("C:\\FILE\\THAT\\DOES\\NOT\\EXIST.TXT");
+#else
+ FileSpec spec("/file/that/does/not/exist.txt");
+#endif
+ FileSystem fs;
+ auto file = fs.Open(spec, File::eOpenOptionRead, 0, true);
+ ASSERT_FALSE(file);
+ std::error_code code = errorToErrorCode(file.takeError());
+ EXPECT_EQ(code.category(), std::system_category());
+ EXPECT_EQ(code.value(), ENOENT);
+}
+
Index: lldb/source/Target/Platform.cpp
===================================================================
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1226,15 +1226,15 @@
if (fs::is_symlink_file(source.GetPath()))
source_open_options |= File::eOpenOptionDontFollowSymlinks;
- File source_file;
- Status error = FileSystem::Instance().Open(
- source_file, source, source_open_options, lldb::eFilePermissionsUserRW);
- uint32_t permissions = source_file.GetPermissions(error);
+ auto source_file = FileSystem::Instance().Open(
+ source, source_open_options, lldb::eFilePermissionsUserRW);
+ if (!source_file)
+ return Status(source_file.takeError());
+ Status error;
+ uint32_t permissions = source_file.get()->GetPermissions(error);
if (permissions == 0)
permissions = lldb::eFilePermissionsFileDefault;
- if (!source_file.IsValid())
- return Status("PutFile: unable to open source file");
lldb::user_id_t dest_file = OpenFile(
destination, File::eOpenOptionCanCreate | File::eOpenOptionWrite |
File::eOpenOptionTruncate | File::eOpenOptionCloseOnExec,
@@ -1249,7 +1249,7 @@
uint64_t offset = 0;
for (;;) {
size_t bytes_read = buffer_sp->GetByteSize();
- error = source_file.Read(buffer_sp->GetBytes(), bytes_read);
+ error = source_file.get()->Read(buffer_sp->GetBytes(), bytes_read);
if (error.Fail() || bytes_read == 0)
break;
@@ -1262,7 +1262,7 @@
if (bytes_written != bytes_read) {
// We didn't write the correct number of bytes, so adjust the file
// position in the source file we are reading from...
- source_file.SeekFromStart(offset);
+ source_file.get()->SeekFromStart(offset);
}
}
CloseFile(dest_file, error);
Index: lldb/source/Target/ModuleCache.cpp
===================================================================
--- lldb/source/Target/ModuleCache.cpp
+++ lldb/source/Target/ModuleCache.cpp
@@ -48,7 +48,7 @@
class ModuleLock {
private:
- File m_file;
+ FileUP m_file_up;
std::unique_ptr<lldb_private::LockFile> m_lock;
FileSpec m_file_spec;
@@ -157,16 +157,19 @@
return;
m_file_spec = JoinPath(lock_dir_spec, uuid.GetAsString().c_str());
- FileSystem::Instance().Open(m_file, m_file_spec,
- File::eOpenOptionWrite |
- File::eOpenOptionCanCreate |
- File::eOpenOptionCloseOnExec);
- if (!m_file) {
- error.SetErrorToErrno();
+
+ auto file = FileSystem::Instance().Open(
+ m_file_spec, File::eOpenOptionWrite | File::eOpenOptionCanCreate |
+ File::eOpenOptionCloseOnExec);
+ if (file)
+ m_file_up = std::move(file.get());
+ else {
+ m_file_up.reset();
+ error = Status(file.takeError());
return;
}
- m_lock.reset(new lldb_private::LockFile(m_file.GetDescriptor()));
+ m_lock.reset(new lldb_private::LockFile(m_file_up->GetDescriptor()));
error = m_lock->WriteLock(0, 1);
if (error.Fail())
error.SetErrorStringWithFormat("Failed to lock file: %s",
@@ -174,10 +177,11 @@
}
void ModuleLock::Delete() {
- if (!m_file)
+ if (!m_file_up)
return;
- m_file.Close();
+ m_file_up->Close();
+ m_file_up.reset();
llvm::sys::fs::remove(m_file_spec.GetPath());
}
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -45,6 +45,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FormatAdapters.h"
#include <memory>
#include <mutex>
@@ -903,17 +904,24 @@
debugger.AdoptTopIOHandlerFilesIfInvalid(input_file_sp, output_file_sp,
error_file_sp);
} else {
- input_file_sp = std::make_shared<StreamFile>();
- FileSystem::Instance().Open(input_file_sp->GetFile(),
+ auto nullin = FileSystem::Instance().Open(
FileSpec(FileSystem::DEV_NULL),
File::eOpenOptionRead);
-
- output_file_sp = std::make_shared<StreamFile>();
- FileSystem::Instance().Open(output_file_sp->GetFile(),
+ auto nullout = FileSystem::Instance().Open(
FileSpec(FileSystem::DEV_NULL),
File::eOpenOptionWrite);
-
- error_file_sp = output_file_sp;
+ if (!nullin) {
+ result->AppendErrorWithFormatv("failed to open /dev/null: {0}\n",
+ llvm::fmt_consume(nullin.takeError()));
+ return false;
+ }
+ if (!nullout) {
+ result->AppendErrorWithFormatv("failed to open /dev/null: {0}\n",
+ llvm::fmt_consume(nullout.takeError()));
+ return false;
+ }
+ input_file_sp = std::make_shared<StreamFile>(std::move(nullin.get()));
+ error_file_sp = output_file_sp = std::make_shared<StreamFile>(std::move(nullout.get()));
}
FILE *in_file = input_file_sp->GetFile().GetStream();
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -455,7 +455,6 @@
public:
PythonFile();
PythonFile(File &file, const char *mode);
- PythonFile(const char *path, const char *mode);
PythonFile(PyRefType type, PyObject *o);
~PythonFile() override;
@@ -469,7 +468,7 @@
static uint32_t GetOptionsFromMode(llvm::StringRef mode);
- bool GetUnderlyingFile(File &file) const;
+ lldb::FileUP GetUnderlyingFile() const;
};
} // namespace lldb_private
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -949,11 +949,6 @@
PythonFile::PythonFile(File &file, const char *mode) { Reset(file, mode); }
-PythonFile::PythonFile(const char *path, const char *mode) {
- lldb_private::File file;
- FileSystem::Instance().Open(file, FileSpec(path), GetOptionsFromMode(mode));
- Reset(file, mode);
-}
PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); }
@@ -1036,17 +1031,19 @@
.Default(0);
}
-bool PythonFile::GetUnderlyingFile(File &file) const {
+FileUP PythonFile::GetUnderlyingFile() const {
if (!IsValid())
- return false;
+ return nullptr;
- file.Close();
// We don't own the file descriptor returned by this function, make sure the
// File object knows about that.
PythonString py_mode = GetAttributeValue("mode").AsType<PythonString>();
auto options = PythonFile::GetOptionsFromMode(py_mode.GetString());
- file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), options, false);
- return file.IsValid();
+ auto file = std::make_unique<File>(PyObject_AsFileDescriptor(m_py_obj),
+ options, false);
+ if (!file->IsValid())
+ return nullptr;
+ return file;
}
#endif
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -99,12 +99,14 @@
// namespace. This allows you to attach with a debugger and call this function
// and get the packet history dumped to a file.
void DumpProcessGDBRemotePacketHistory(void *p, const char *path) {
- StreamFile strm;
- Status error = FileSystem::Instance().Open(strm.GetFile(), FileSpec(path),
- File::eOpenOptionWrite |
- File::eOpenOptionCanCreate);
- if (error.Success())
- ((ProcessGDBRemote *)p)->GetGDBRemote().DumpHistory(strm);
+ auto file = FileSystem::Instance().Open(
+ FileSpec(path), File::eOpenOptionWrite | File::eOpenOptionCanCreate);
+ if (!file) {
+ llvm::consumeError(file.takeError());
+ return;
+ }
+ StreamFile stream(std::move(file.get()));
+ ((ProcessGDBRemote *)p)->GetGDBRemote().DumpHistory(stream);
}
} // namespace lldb
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -512,14 +512,23 @@
mode_t mode = packet.GetHexMaxU32(false, 0600);
FileSpec path_spec(path);
FileSystem::Instance().Resolve(path_spec);
- File file;
// Do not close fd.
- Status error =
- FileSystem::Instance().Open(file, path_spec, flags, mode, false);
- const int save_errno = error.GetError();
+ auto file = FileSystem::Instance().Open(path_spec, flags, mode, false);
+
+ int save_errno = 0;
+ int descriptor = File::kInvalidDescriptor;
+ if (file) {
+ descriptor = file.get()->GetDescriptor();
+ } else {
+ std::error_code code = errorToErrorCode(file.takeError());
+ if (code.category() == std::system_category()) {
+ save_errno = code.value();
+ }
+ }
+
StreamString response;
response.PutChar('F');
- response.Printf("%i", file.GetDescriptor());
+ response.Printf("%i", descriptor);
if (save_errno)
response.Printf(",%i", save_errno);
return SendPacketNoLock(response.GetString());
Index: lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===================================================================
--- lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -382,7 +382,7 @@
static Status HandleFileAction(ProcessLaunchInfo &launch_info,
NSMutableDictionary *options, NSString *key,
- const int fd, File &file) {
+ const int fd, lldb::FileSP &file) {
Status error;
const FileAction *file_action = launch_info.GetFileActionForFD(fd);
if (file_action) {
@@ -434,7 +434,7 @@
file_options |= File::eOpenOptionRead;
if ((oflag & O_RDWR) || (oflag & O_RDONLY))
file_options |= File::eOpenOptionWrite;
- file.SetDescriptor(created_fd, file_options, true);
+ file = std::make_shared<File>(created_fd, file_options, true);
[options setValue:[NSNumber numberWithInteger:created_fd] forKey:key];
return error; // Success
} else {
@@ -494,9 +494,9 @@
[options setObject:env_dict forKey:kSimDeviceSpawnEnvironment];
Status error;
- File stdin_file;
- File stdout_file;
- File stderr_file;
+ lldb::FileSP stdin_file;
+ lldb::FileSP stdout_file;
+ lldb::FileSP stderr_file;
error = HandleFileAction(launch_info, options, kSimDeviceSpawnStdin,
STDIN_FILENO, stdin_file);
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6271,22 +6271,23 @@
buffer.PutHex32(segment.flags);
}
- File core_file;
std::string core_file_path(outfile.GetPath());
- error = FileSystem::Instance().Open(core_file, outfile,
- File::eOpenOptionWrite |
- File::eOpenOptionTruncate |
- File::eOpenOptionCanCreate);
- if (error.Success()) {
+ auto core_file = FileSystem::Instance().Open(
+ outfile, File::eOpenOptionWrite | File::eOpenOptionTruncate |
+ File::eOpenOptionCanCreate);
+ if (!core_file) {
+ error = core_file.takeError();
+ } else {
// Read 1 page at a time
uint8_t bytes[0x1000];
// Write the mach header and load commands out to the core file
size_t bytes_written = buffer.GetString().size();
- error = core_file.Write(buffer.GetString().data(), bytes_written);
+ error = core_file.get()->Write(buffer.GetString().data(),
+ bytes_written);
if (error.Success()) {
// Now write the file data for all memory segments in the process
for (const auto &segment : segment_load_commands) {
- if (core_file.SeekFromStart(segment.fileoff) == -1) {
+ if (core_file.get()->SeekFromStart(segment.fileoff) == -1) {
error.SetErrorStringWithFormat(
"unable to seek to offset 0x%" PRIx64 " in '%s'",
segment.fileoff, core_file_path.c_str());
@@ -6311,7 +6312,7 @@
if (bytes_read == bytes_to_read) {
size_t bytes_written = bytes_read;
- error = core_file.Write(bytes, bytes_written);
+ error = core_file.get()->Write(bytes, bytes_written);
bytes_left -= bytes_read;
addr += bytes_read;
} else {
@@ -6319,7 +6320,7 @@
// be zero filled
memset(bytes, 0, bytes_to_read);
size_t bytes_written = bytes_to_read;
- error = core_file.Write(bytes, bytes_written);
+ error = core_file.get()->Write(bytes, bytes_written);
bytes_left -= bytes_to_read;
addr += bytes_to_read;
}
Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -2651,14 +2651,14 @@
// Check we can create writable file
FileSpec file_spec(path);
FileSystem::Instance().Resolve(file_spec);
- File file;
- FileSystem::Instance().Open(file, file_spec,
- File::eOpenOptionWrite |
- File::eOpenOptionCanCreate |
- File::eOpenOptionTruncate);
+ auto file = FileSystem::Instance().Open(
+ file_spec, File::eOpenOptionWrite | File::eOpenOptionCanCreate |
+ File::eOpenOptionTruncate);
if (!file) {
- strm.Printf("Error: Failed to open '%s' for writing", path);
+ std::string error = llvm::toString(file.takeError());
+ strm.Printf("Error: Failed to open '%s' for writing: %s", path,
+ error.c_str());
strm.EOL();
return false;
}
@@ -2690,7 +2690,7 @@
LLDB_LOGF(log, "%s - writing File Header, 0x%" PRIx64 " bytes", __FUNCTION__,
(uint64_t)num_bytes);
- Status err = file.Write(&head, num_bytes);
+ Status err = file.get()->Write(&head, num_bytes);
if (!err.Success()) {
strm.Printf("Error: '%s' when writing to file '%s'", err.AsCString(), path);
strm.EOL();
@@ -2715,7 +2715,7 @@
LLDB_LOGF(log, "%s - writing element headers, 0x%" PRIx64 " bytes.",
__FUNCTION__, (uint64_t)num_bytes);
- err = file.Write(element_header_buffer.get(), num_bytes);
+ err = file.get()->Write(element_header_buffer.get(), num_bytes);
if (!err.Success()) {
strm.Printf("Error: '%s' when writing to file '%s'", err.AsCString(), path);
strm.EOL();
@@ -2727,7 +2727,7 @@
LLDB_LOGF(log, "%s - writing 0x%" PRIx64 " bytes", __FUNCTION__,
(uint64_t)num_bytes);
- err = file.Write(buffer.get(), num_bytes);
+ err = file.get()->Write(buffer.get(), num_bytes);
if (!err.Success()) {
strm.Printf("Error: '%s' when writing to file '%s'", err.AsCString(), path);
strm.EOL();
@@ -4578,32 +4578,36 @@
return false;
}
- Stream *output_strm = nullptr;
- StreamFile outfile_stream;
+ Stream *output_stream_p = nullptr;
+ std::unique_ptr<Stream> output_stream_storage;
+
const FileSpec &outfile_spec =
m_options.m_outfile; // Dump allocation to file instead
if (outfile_spec) {
// Open output file
std::string path = outfile_spec.GetPath();
- auto error = FileSystem::Instance().Open(
- outfile_stream.GetFile(), outfile_spec,
- File::eOpenOptionWrite | File::eOpenOptionCanCreate);
- if (error.Success()) {
- output_strm = &outfile_stream;
+ auto file = FileSystem::Instance().Open(
+ outfile_spec, File::eOpenOptionWrite | File::eOpenOptionCanCreate);
+ if (file) {
+ output_stream_storage =
+ std::make_unique<StreamFile>(std::move(file.get()));
+ output_stream_p = output_stream_storage.get();
result.GetOutputStream().Printf("Results written to '%s'",
path.c_str());
result.GetOutputStream().EOL();
} else {
- result.AppendErrorWithFormat("Couldn't open file '%s'", path.c_str());
+ std::string error = llvm::toString(file.takeError());
+ result.AppendErrorWithFormat("Couldn't open file '%s': %s",
+ path.c_str(), error.c_str());
result.SetStatus(eReturnStatusFailed);
return false;
}
} else
- output_strm = &result.GetOutputStream();
+ output_stream_p = &result.GetOutputStream();
- assert(output_strm != nullptr);
+ assert(output_stream_p != nullptr);
bool dumped =
- runtime->DumpAllocation(*output_strm, m_exe_ctx.GetFramePtr(), id);
+ runtime->DumpAllocation(*output_stream_p, m_exe_ctx.GetFramePtr(), id);
if (dumped)
result.SetStatus(eReturnStatusSuccessFinishResult);
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -68,6 +68,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FormatAdapters.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/PrettyStackTrace.h"
@@ -2315,18 +2316,17 @@
return;
}
- StreamFileSP input_file_sp(new StreamFile());
std::string cmd_file_path = cmd_file.GetPath();
- Status error = FileSystem::Instance().Open(input_file_sp->GetFile(), cmd_file,
- File::eOpenOptionRead);
- if (error.Fail()) {
- result.AppendErrorWithFormat(
- "error: an error occurred read file '%s': %s\n", cmd_file_path.c_str(),
- error.AsCString());
+ auto file = FileSystem::Instance().Open(cmd_file, File::eOpenOptionRead);
+ if (!file) {
+ result.AppendErrorWithFormatv(
+ "error: an error occurred read file '{0}': {1}\n", cmd_file_path,
+ llvm::fmt_consume(file.takeError()));
result.SetStatus(eReturnStatusFailed);
return;
}
+ auto input_file_sp = std::make_shared<StreamFile>(std::move(file.get()));
Debugger &debugger = GetDebugger();
Index: lldb/source/Host/windows/Host.cpp
===================================================================
--- lldb/source/Host/windows/Host.cpp
+++ lldb/source/Host/windows/Host.cpp
@@ -34,9 +34,11 @@
bool GetTripleForProcess(const FileSpec &executable, llvm::Triple &triple) {
// Open the PE File as a binary file, and parse just enough information to
// determine the machine type.
- File imageBinary;
- FileSystem::Instance().Open(imageBinary, executable, File::eOpenOptionRead,
- lldb::eFilePermissionsUserRead);
+ auto imageBinaryP = FileSystem::Instance().Open(
+ executable, File::eOpenOptionRead, lldb::eFilePermissionsUserRead);
+ if (!imageBinaryP)
+ return llvm::errorToBool(imageBinaryP.takeError());
+ File &imageBinary = *imageBinaryP.get();
imageBinary.SeekFromStart(0x3c);
int32_t peOffset = 0;
uint32_t peHead = 0;
Index: lldb/source/Host/common/FileSystem.cpp
===================================================================
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -415,33 +415,29 @@
return mode;
}
-Status FileSystem::Open(File &File, const FileSpec &file_spec, uint32_t options,
- uint32_t permissions, bool should_close_fd) {
+Expected<FileUP> FileSystem::Open(const FileSpec &file_spec, uint32_t options,
+ uint32_t permissions, bool should_close_fd) {
if (m_collector)
m_collector->addFile(file_spec.GetPath());
- if (File.IsValid())
- File.Close();
-
const int open_flags = GetOpenFlags(options);
const mode_t open_mode =
(open_flags & O_CREAT) ? GetOpenMode(permissions) : 0;
auto path = GetExternalPath(file_spec);
if (!path)
- return Status(path.getError());
+ return errorCodeToError(path.getError());
int descriptor = llvm::sys::RetryAfterSignal(
-1, OpenWithFS, *this, path->c_str(), open_flags, open_mode);
- Status error;
- if (!File::DescriptorIsValid(descriptor)) {
- File.SetDescriptor(-1, options, false);
- error.SetErrorToErrno();
- } else {
- File.SetDescriptor(descriptor, options, should_close_fd);
- }
- return error;
+ if (!File::DescriptorIsValid(descriptor))
+ return llvm::errorCodeToError(
+ std::error_code(errno, std::system_category()));
+
+ auto file = std::make_unique<File>(descriptor, options, should_close_fd);
+ assert(file->IsValid());
+ return std::move(file);
}
ErrorOr<std::string> FileSystem::GetExternalPath(const llvm::Twine &path) {
Index: lldb/source/Host/common/FileCache.cpp
===================================================================
--- lldb/source/Host/common/FileCache.cpp
+++ lldb/source/Host/common/FileCache.cpp
@@ -29,12 +29,13 @@
error.SetErrorString("empty path");
return UINT64_MAX;
}
- FileSP file_sp(new File());
- error = FileSystem::Instance().Open(*file_sp, file_spec, flags, mode);
- if (!file_sp->IsValid())
+ auto file = FileSystem::Instance().Open(file_spec, flags, mode);
+ if (!file) {
+ error = file.takeError();
return UINT64_MAX;
- lldb::user_id_t fd = file_sp->GetDescriptor();
- m_cache[fd] = file_sp;
+ }
+ lldb::user_id_t fd = file.get()->GetDescriptor();
+ m_cache[fd] = std::move(file.get());
return fd;
}
@@ -48,12 +49,12 @@
error.SetErrorStringWithFormat("invalid host file descriptor %" PRIu64, fd);
return false;
}
- FileSP file_sp = pos->second;
- if (!file_sp) {
+ FileUP &file_up = pos->second;
+ if (!file_up) {
error.SetErrorString("invalid host backing file");
return false;
}
- error = file_sp->Close();
+ error = file_up->Close();
m_cache.erase(pos);
return error.Success();
}
@@ -70,16 +71,16 @@
error.SetErrorStringWithFormat("invalid host file descriptor %" PRIu64, fd);
return false;
}
- FileSP file_sp = pos->second;
- if (!file_sp) {
+ FileUP &file_up = pos->second;
+ if (!file_up) {
error.SetErrorString("invalid host backing file");
return UINT64_MAX;
}
- if (static_cast<uint64_t>(file_sp->SeekFromStart(offset, &error)) != offset ||
+ if (static_cast<uint64_t>(file_up->SeekFromStart(offset, &error)) != offset ||
error.Fail())
return UINT64_MAX;
size_t bytes_written = src_len;
- error = file_sp->Write(src, bytes_written);
+ error = file_up->Write(src, bytes_written);
if (error.Fail())
return UINT64_MAX;
return bytes_written;
@@ -96,16 +97,16 @@
error.SetErrorStringWithFormat("invalid host file descriptor %" PRIu64, fd);
return false;
}
- FileSP file_sp = pos->second;
- if (!file_sp) {
+ FileUP &file_up = pos->second;
+ if (!file_up) {
error.SetErrorString("invalid host backing file");
return UINT64_MAX;
}
- if (static_cast<uint64_t>(file_sp->SeekFromStart(offset, &error)) != offset ||
+ if (static_cast<uint64_t>(file_up->SeekFromStart(offset, &error)) != offset ||
error.Fail())
return UINT64_MAX;
size_t bytes_read = dst_len;
- error = file_sp->Read(dst, bytes_read);
+ error = file_up->Read(dst, bytes_read);
if (error.Fail())
return UINT64_MAX;
return bytes_read;
Index: lldb/source/Expression/REPL.cpp
===================================================================
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -398,17 +398,22 @@
// Update our code on disk
if (!m_repl_source_path.empty()) {
- lldb_private::File file;
- FileSystem::Instance().Open(file, FileSpec(m_repl_source_path),
+ auto file = FileSystem::Instance().Open(FileSpec(m_repl_source_path),
File::eOpenOptionWrite |
File::eOpenOptionTruncate |
File::eOpenOptionCanCreate,
lldb::eFilePermissionsFileDefault);
- std::string code(m_code.CopyList());
- code.append(1, '\n');
- size_t bytes_written = code.size();
- file.Write(code.c_str(), bytes_written);
- file.Close();
+ if (file) {
+ std::string code(m_code.CopyList());
+ code.append(1, '\n');
+ size_t bytes_written = code.size();
+ file.get()->Write(code.c_str(), bytes_written);
+ file.get()->Close();
+ } else {
+ std::string message = llvm::toString(file.takeError());
+ error_sp->Printf("error: couldn't open %s: %s\n",
+ m_repl_source_path.c_str(), message.c_str());
+ }
// Now set the default file and line to the REPL source file
m_target.GetSourceManager().SetDefaultFileAndLine(
Index: lldb/source/Core/StreamFile.cpp
===================================================================
--- lldb/source/Core/StreamFile.cpp
+++ lldb/source/Core/StreamFile.cpp
@@ -8,6 +8,7 @@
#include "lldb/Core/StreamFile.h"
#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/Log.h"
#include <stdio.h>
@@ -15,35 +16,54 @@
using namespace lldb_private;
// StreamFile constructor
-StreamFile::StreamFile() : Stream(), m_file() {}
+StreamFile::StreamFile() : Stream() { m_file_sp = std::make_shared<File>(); }
StreamFile::StreamFile(uint32_t flags, uint32_t addr_size, ByteOrder byte_order)
- : Stream(flags, addr_size, byte_order), m_file() {}
+ : Stream(flags, addr_size, byte_order) {
+ m_file_sp = std::make_shared<File>();
+}
-StreamFile::StreamFile(int fd, bool transfer_ownership)
- : Stream(), m_file(fd, File::eOpenOptionWrite, transfer_ownership) {}
+StreamFile::StreamFile(int fd, bool transfer_ownership) : Stream() {
+ m_file_sp =
+ std::make_shared<File>(fd, File::eOpenOptionWrite, transfer_ownership);
+}
-StreamFile::StreamFile(FILE *fh, bool transfer_ownership)
- : Stream(), m_file(fh, transfer_ownership) {}
+StreamFile::StreamFile(FILE *fh, bool transfer_ownership) : Stream() {
+ m_file_sp = std::make_shared<File>(fh, transfer_ownership);
+}
-StreamFile::StreamFile(const char *path) : Stream(), m_file() {
- FileSystem::Instance().Open(m_file, FileSpec(path),
- File::eOpenOptionWrite |
- File::eOpenOptionCanCreate |
- File::eOpenOptionCloseOnExec);
+StreamFile::StreamFile(const char *path) : Stream() {
+ auto file = FileSystem::Instance().Open(
+ FileSpec(path), File::eOpenOptionWrite | File::eOpenOptionCanCreate |
+ File::eOpenOptionCloseOnExec);
+ if (file)
+ m_file_sp = std::move(file.get());
+ else {
+ // TODO refactor this so the error gets popagated up instead of logged here.
+ LLDB_LOG_ERROR(GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST), file.takeError(),
+ "Cannot open {1}: {0}", path);
+ m_file_sp = std::make_shared<File>();
+ }
}
StreamFile::StreamFile(const char *path, uint32_t options, uint32_t permissions)
- : Stream(), m_file() {
-
- FileSystem::Instance().Open(m_file, FileSpec(path), options, permissions);
+ : Stream() {
+ auto file = FileSystem::Instance().Open(FileSpec(path), options, permissions);
+ if (file)
+ m_file_sp = std::move(file.get());
+ else {
+ // TODO refactor this so the error gets popagated up instead of logged here.
+ LLDB_LOG_ERROR(GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST), file.takeError(),
+ "Cannot open {1}: {0}", path);
+ m_file_sp = std::make_shared<File>();
+ }
}
StreamFile::~StreamFile() {}
-void StreamFile::Flush() { m_file.Flush(); }
+void StreamFile::Flush() { m_file_sp->Flush(); }
size_t StreamFile::WriteImpl(const void *s, size_t length) {
- m_file.Write(s, length);
+ m_file_sp->Write(s, length);
return length;
}
Index: lldb/source/Commands/CommandObjectMemory.cpp
===================================================================
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -765,8 +765,8 @@
m_prev_varobj_options = m_varobj_options;
m_prev_compiler_type = compiler_type;
- StreamFile outfile_stream;
- Stream *output_stream = nullptr;
+ std::unique_ptr<Stream> output_stream_storage;
+ Stream *output_stream_p = nullptr;
const FileSpec &outfile_spec =
m_outfile_options.GetFile().GetCurrentValue();
@@ -779,12 +779,14 @@
if (append)
open_options |= File::eOpenOptionAppend;
- Status error = FileSystem::Instance().Open(outfile_stream.GetFile(),
- outfile_spec, open_options);
- if (error.Success()) {
+ auto outfile = FileSystem::Instance().Open(outfile_spec, open_options);
+
+ if (outfile) {
+ auto outfile_stream_up =
+ std::make_unique<StreamFile>(std::move(outfile.get()));
if (m_memory_options.m_output_as_binary) {
const size_t bytes_written =
- outfile_stream.Write(data_sp->GetBytes(), bytes_read);
+ outfile_stream_up->Write(data_sp->GetBytes(), bytes_read);
if (bytes_written > 0) {
result.GetOutputStream().Printf(
"%zi bytes %s to '%s'\n", bytes_written,
@@ -800,16 +802,19 @@
} else {
// We are going to write ASCII to the file just point the
// output_stream to our outfile_stream...
- output_stream = &outfile_stream;
+ output_stream_storage = std::move(outfile_stream_up);
+ output_stream_p = output_stream_storage.get();
}
} else {
- result.AppendErrorWithFormat("Failed to open file '%s' for %s.\n",
+ result.AppendErrorWithFormat("Failed to open file '%s' for %s:\n",
path.c_str(), append ? "append" : "write");
+
+ result.AppendError(llvm::toString(outfile.takeError()));
result.SetStatus(eReturnStatusFailed);
return false;
}
} else {
- output_stream = &result.GetOutputStream();
+ output_stream_p = &result.GetOutputStream();
}
ExecutionContextScope *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
@@ -829,7 +834,7 @@
DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions(
eLanguageRuntimeDescriptionDisplayVerbosityFull, format));
- valobj_sp->Dump(*output_stream, options);
+ valobj_sp->Dump(*output_stream_p, options);
} else {
result.AppendErrorWithFormat(
"failed to create a value object for: (%s) %s\n",
@@ -869,13 +874,13 @@
}
}
- assert(output_stream);
+ assert(output_stream_p);
size_t bytes_dumped = DumpDataExtractor(
- data, output_stream, 0, format, item_byte_size, item_count,
+ data, output_stream_p, 0, format, item_byte_size, item_count,
num_per_line / target->GetArchitecture().GetDataByteSize(), addr, 0, 0,
exe_scope);
m_next_addr = addr + bytes_dumped;
- output_stream->EOL();
+ output_stream_p->EOL();
return true;
}
Index: lldb/source/API/SBStream.cpp
===================================================================
--- lldb/source/API/SBStream.cpp
+++ lldb/source/API/SBStream.cpp
@@ -82,26 +82,27 @@
if (!m_is_file)
local_data = static_cast<StreamString *>(m_opaque_up.get())->GetString();
}
- StreamFile *stream_file = new StreamFile;
uint32_t open_options = File::eOpenOptionWrite | File::eOpenOptionCanCreate;
if (append)
open_options |= File::eOpenOptionAppend;
else
open_options |= File::eOpenOptionTruncate;
- FileSystem::Instance().Open(stream_file->GetFile(), FileSpec(path),
- open_options);
- m_opaque_up.reset(stream_file);
+ llvm::Expected<FileUP> file =
+ FileSystem::Instance().Open(FileSpec(path), open_options);
+ if (!file) {
+ LLDB_LOG_ERROR(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), file.takeError(),
+ "Cannot open {1}: {0}", path);
+ return;
+ }
- if (m_opaque_up) {
- m_is_file = true;
-
- // If we had any data locally in our StreamString, then pass that along to
- // the to new file we are redirecting to.
- if (!local_data.empty())
- m_opaque_up->Write(&local_data[0], local_data.size());
- } else
- m_is_file = false;
+ m_opaque_up = std::make_unique<StreamFile>(std::move(file.get()));
+ m_is_file = true;
+
+ // If we had any data locally in our StreamString, then pass that along to
+ // the to new file we are redirecting to.
+ if (!local_data.empty())
+ m_opaque_up->Write(&local_data[0], local_data.size());
}
void SBStream::RedirectToFileHandle(FILE *fh, bool transfer_fh_ownership) {
@@ -118,17 +119,14 @@
if (!m_is_file)
local_data = static_cast<StreamString *>(m_opaque_up.get())->GetString();
}
- m_opaque_up.reset(new StreamFile(fh, transfer_fh_ownership));
- if (m_opaque_up) {
- m_is_file = true;
-
- // If we had any data locally in our StreamString, then pass that along to
- // the to new file we are redirecting to.
- if (!local_data.empty())
- m_opaque_up->Write(&local_data[0], local_data.size());
- } else
- m_is_file = false;
+ m_opaque_up = std::make_unique<StreamFile>(fh, transfer_fh_ownership);
+ m_is_file = true;
+
+ // If we had any data locally in our StreamString, then pass that along to
+ // the to new file we are redirecting to.
+ if (!local_data.empty())
+ m_opaque_up->Write(&local_data[0], local_data.size());
}
void SBStream::RedirectToFileDescriptor(int fd, bool transfer_fh_ownership) {
@@ -143,16 +141,13 @@
local_data = static_cast<StreamString *>(m_opaque_up.get())->GetString();
}
- m_opaque_up.reset(new StreamFile(::fdopen(fd, "w"), transfer_fh_ownership));
- if (m_opaque_up) {
- m_is_file = true;
-
- // If we had any data locally in our StreamString, then pass that along to
- // the to new file we are redirecting to.
- if (!local_data.empty())
- m_opaque_up->Write(&local_data[0], local_data.size());
- } else
- m_is_file = false;
+ m_opaque_up = std::make_unique<StreamFile>(fd, transfer_fh_ownership);
+ m_is_file = true;
+
+ // If we had any data locally in our StreamString, then pass that along to
+ // the to new file we are redirecting to.
+ if (!local_data.empty())
+ m_opaque_up->Write(&local_data[0], local_data.size());
}
lldb_private::Stream *SBStream::operator->() { return m_opaque_up.get(); }
Index: lldb/scripts/Python/python-typemaps.swig
===================================================================
--- lldb/scripts/Python/python-typemaps.swig
+++ lldb/scripts/Python/python-typemaps.swig
@@ -395,13 +395,12 @@
else
{
PythonFile py_file(PyRefType::Borrowed, $input);
- File file;
- if (!py_file.GetUnderlyingFile(file))
+ lldb::FileUP file = py_file.GetUnderlyingFile();
+ if (!file)
return nullptr;
-
- $1 = file.GetStream();
+ $1 = file->GetStream();
if ($1)
- file.Clear();
+ file->Clear();
}
}
Index: lldb/include/lldb/lldb-forward.h
===================================================================
--- lldb/include/lldb/lldb-forward.h
+++ lldb/include/lldb/lldb-forward.h
@@ -331,6 +331,7 @@
typedef std::shared_ptr<lldb_private::ExecutionContextRef>
ExecutionContextRefSP;
typedef std::shared_ptr<lldb_private::ExpressionVariable> ExpressionVariableSP;
+typedef std::unique_ptr<lldb_private::File> FileUP;
typedef std::shared_ptr<lldb_private::File> FileSP;
typedef std::shared_ptr<lldb_private::Function> FunctionSP;
typedef std::shared_ptr<lldb_private::FunctionCaller> FunctionCallerSP;
Index: lldb/include/lldb/Host/FileSystem.h
===================================================================
--- lldb/include/lldb/Host/FileSystem.h
+++ lldb/include/lldb/Host/FileSystem.h
@@ -63,9 +63,10 @@
/// Wraps ::open in a platform-independent way.
int Open(const char *path, int flags, int mode);
- Status Open(File &File, const FileSpec &file_spec, uint32_t options,
- uint32_t permissions = lldb::eFilePermissionsFileDefault,
- bool should_close_fd = true);
+ llvm::Expected<std::unique_ptr<File>>
+ Open(const FileSpec &file_spec, uint32_t options,
+ uint32_t permissions = lldb::eFilePermissionsFileDefault,
+ bool should_close_fd = true);
/// Get a directory iterator.
/// \{
Index: lldb/include/lldb/Host/FileCache.h
===================================================================
--- lldb/include/lldb/Host/FileCache.h
+++ lldb/include/lldb/Host/FileCache.h
@@ -22,7 +22,7 @@
private:
FileCache() {}
- typedef std::map<lldb::user_id_t, lldb::FileSP> FDToFileMap;
+ typedef std::map<lldb::user_id_t, lldb::FileUP> FDToFileMap;
public:
static FileCache &GetInstance();
Index: lldb/include/lldb/Core/StreamFile.h
===================================================================
--- lldb/include/lldb/Core/StreamFile.h
+++ lldb/include/lldb/Core/StreamFile.h
@@ -35,18 +35,22 @@
StreamFile(FILE *fh, bool transfer_ownership);
+ StreamFile(std::shared_ptr<File> file) : m_file_sp(file) { assert(file); };
+
~StreamFile() override;
- File &GetFile() { return m_file; }
+ File &GetFile() { return *m_file_sp; }
+
+ const File &GetFile() const { return *m_file_sp; }
- const File &GetFile() const { return m_file; }
+ std::shared_ptr<File> GetFileSP() { return m_file_sp; }
void Flush() override;
protected:
// Classes that inherit from StreamFile can see and modify these
- File m_file;
+ std::shared_ptr<File> m_file_sp; // never NULL
size_t WriteImpl(const void *s, size_t length) override;
private:
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
