diff --git a/tools/crashpad_database_util.cc b/tools/crashpad_database_util.cc index 4a2dcc65..b2c4cb49 100644 --- a/tools/crashpad_database_util.cc +++ b/tools/crashpad_database_util.cc @@ -560,13 +560,21 @@ int DatabaseUtilMain(int argc, char* argv[]) { return EXIT_FAILURE; } + bool used_stdin = false; for (const base::FilePath new_report_path : options.new_report_paths) { std::unique_ptr file_reader; - bool is_stdin = false; if (new_report_path.value() == FILE_PATH_LITERAL("-")) { - is_stdin = true; - file_reader.reset(new WeakStdioFileReader(stdin)); + if (used_stdin) { + fprintf(stderr, + "%" PRFilePath + ": Only one --new-report may be read from standard input\n", + me.value().c_str()); + return EXIT_FAILURE; + } + used_stdin = true; + file_reader.reset(new WeakFileHandleFileReader( + StdioFileHandle(StdioStream::kStandardInput))); } else { std::unique_ptr file_path_reader(new FileReader()); if (!file_path_reader->Open(new_report_path)) { @@ -607,13 +615,6 @@ int DatabaseUtilMain(int argc, char* argv[]) { return EXIT_FAILURE; } - file_reader.reset(); - if (is_stdin) { - if (fclose(stdin) == EOF) { - STDIO_PLOG(ERROR) << "fclose"; - } - } - const char* prefix = (show_operations > 1) ? "New report ID: " : ""; printf("%s%s\n", prefix, uuid.ToString().c_str()); } diff --git a/tools/crashpad_http_upload.cc b/tools/crashpad_http_upload.cc index 55ab9550..d4ec3f9c 100644 --- a/tools/crashpad_http_upload.cc +++ b/tools/crashpad_http_upload.cc @@ -163,7 +163,8 @@ int HTTPUploadMain(int argc, char* argv[]) { return EXIT_FAILURE; } } else { - file_writer.reset(new WeakStdioFileWriter(stdout)); + file_writer.reset(new WeakFileHandleFileWriter( + StdioFileHandle(StdioStream::kStandardOutput))); } http_multipart_builder.SetGzipEnabled(options.upload_gzip); diff --git a/util/file/file_io.h b/util/file/file_io.h index a7415b8c..a720cdda 100644 --- a/util/file/file_io.h +++ b/util/file/file_io.h @@ -26,27 +26,6 @@ #include "util/win/scoped_handle.h" #endif -//! \file - -#if defined(OS_POSIX) || DOXYGEN - -//! \brief A `PLOG()` macro usable for standard input/output error conditions. -//! -//! The `PLOG()` macro uses `errno` on POSIX and is appropriate to report -//! errors from standard input/output functions. On Windows, `PLOG()` uses -//! `GetLastError()`, and cannot be used to report errors from standard -//! input/output functions. This macro uses `PLOG()` when appropriate for -//! standard I/O functions, and `LOG()` otherwise. -#define STDIO_PLOG(x) PLOG(x) - -#else - -#define STDIO_PLOG(x) LOG(x) -#define fseeko(file, offset, whence) _fseeki64(file, offset, whence) -#define ftello(file) _ftelli64(file) - -#endif - namespace base { class FilePath; } // namespace base @@ -114,6 +93,18 @@ enum class FileLocking : bool { kExclusive, }; +//! \brief Determines the FileHandle that StdioFileHandle() returns. +enum class StdioStream { + //! \brief Standard input, or `stdin`. + kStandardInput, + + //! \brief Standard output, or `stdout`. + kStandardOutput, + + //! \brief Standard error, or `stderr`. + kStandardError, +}; + //! \brief The name of the native read function used by ReadFile(). //! //! This value may be useful for logging. @@ -381,12 +372,27 @@ void CheckedCloseFile(FileHandle file); //! \brief Determines the size of a file. //! //! \param[in] file The handle to the file for which the size should be -//! retrived. +//! retrieved. //! //! \return The size of the file. If an error occurs when attempting to //! determine its size, returns `-1` with an error logged. FileOffset LoggingFileSizeByHandle(FileHandle file); +//! \brief Returns a FileHandle corresponding to the requested standard I/O +//! stream. +//! +//! The returned FileHandle should not be closed on POSIX, where it is +//! important to maintain valid file descriptors occupying the slots reserved +//! for these streams. If a need to close such a stream arises on POSIX, +//! `dup2()` should instead be used to replace the existing file descriptor with +//! one opened to `/dev/null`. See CloseStdinAndStdout(). +//! +//! \param[in] stdio_stream The requested standard I/O stream. +//! +//! \return A corresponding FileHandle on success. kInvalidFileHandle on error, +//! with a message logged. +FileHandle StdioFileHandle(StdioStream stdio_stream); + } // namespace crashpad #endif // CRASHPAD_UTIL_FILE_FILE_IO_H_ diff --git a/util/file/file_io_posix.cc b/util/file/file_io_posix.cc index 00d14cd2..79543e3f 100644 --- a/util/file/file_io_posix.cc +++ b/util/file/file_io_posix.cc @@ -181,4 +181,15 @@ FileOffset LoggingFileSizeByHandle(FileHandle file) { return st.st_size; } +FileHandle StdioFileHandle(StdioStream stdio_stream) { + switch (stdio_stream) { + case StdioStream::kStandardInput: + return STDIN_FILENO; + case StdioStream::kStandardOutput: + return STDOUT_FILENO; + case StdioStream::kStandardError: + return STDERR_FILENO; + } +} + } // namespace crashpad diff --git a/util/file/file_io_test.cc b/util/file/file_io_test.cc index b8444d41..989d54d7 100644 --- a/util/file/file_io_test.cc +++ b/util/file/file_io_test.cc @@ -14,6 +14,8 @@ #include "util/file/file_io.h" +#include + #include "base/atomicops.h" #include "base/files/file_path.h" #include "base/macros.h" @@ -324,6 +326,26 @@ TEST(FileIO, FileSizeByHandle) { EXPECT_EQ(9, LoggingFileSizeByHandle(file_handle.get())); } +FileHandle FileHandleForFILE(FILE* file) { + int fd = fileno(file); +#if defined(OS_POSIX) + return fd; +#elif defined(OS_WIN) + return reinterpret_cast(_get_osfhandle(fd)); +#else +#error Port +#endif +} + +TEST(FileIO, StdioFileHandle) { + EXPECT_EQ(FileHandleForFILE(stdin), + StdioFileHandle(StdioStream::kStandardInput)); + EXPECT_EQ(FileHandleForFILE(stdout), + StdioFileHandle(StdioStream::kStandardOutput)); + EXPECT_EQ(FileHandleForFILE(stderr), + StdioFileHandle(StdioStream::kStandardError)); +} + } // namespace } // namespace test } // namespace crashpad diff --git a/util/file/file_io_win.cc b/util/file/file_io_win.cc index 110a5c6a..189dc44d 100644 --- a/util/file/file_io_win.cc +++ b/util/file/file_io_win.cc @@ -246,4 +246,26 @@ FileOffset LoggingFileSizeByHandle(FileHandle file) { return file_size.QuadPart; } +FileHandle StdioFileHandle(StdioStream stdio_stream) { + DWORD standard_handle; + switch (stdio_stream) { + case StdioStream::kStandardInput: + standard_handle = STD_INPUT_HANDLE; + break; + case StdioStream::kStandardOutput: + standard_handle = STD_OUTPUT_HANDLE; + break; + case StdioStream::kStandardError: + standard_handle = STD_ERROR_HANDLE; + break; + default: + NOTREACHED(); + return INVALID_HANDLE_VALUE; + } + + HANDLE handle = GetStdHandle(standard_handle); + PLOG_IF(ERROR, handle == INVALID_HANDLE_VALUE) << "GetStdHandle"; + return handle; +} + } // namespace crashpad diff --git a/util/file/file_reader.cc b/util/file/file_reader.cc index ac625b87..b7fcffb5 100644 --- a/util/file/file_reader.cc +++ b/util/file/file_reader.cc @@ -113,43 +113,4 @@ FileOffset FileReader::Seek(FileOffset offset, int whence) { return weak_file_handle_file_reader_.Seek(offset, whence); } -WeakStdioFileReader::WeakStdioFileReader(FILE* file) - : file_(file) { -} - -WeakStdioFileReader::~WeakStdioFileReader() { -} - -FileOperationResult WeakStdioFileReader::Read(void* data, size_t size) { - DCHECK(file_); - - size_t rv = fread(data, 1, size, file_); - if (rv != size && ferror(file_)) { - STDIO_PLOG(ERROR) << "fread"; - return -1; - } - if (rv > size) { - LOG(ERROR) << "fread: expected " << size << ", observed " << rv; - return -1; - } - - return rv; -} - -FileOffset WeakStdioFileReader::Seek(FileOffset offset, int whence) { - DCHECK(file_); - if (fseeko(file_, offset, whence) == -1) { - STDIO_PLOG(ERROR) << "fseeko"; - return -1; - } - - FileOffset new_offset = ftello(file_); - if (new_offset == -1) { - STDIO_PLOG(ERROR) << "ftello"; - return -1; - } - - return new_offset; -} - } // namespace crashpad diff --git a/util/file/file_reader.h b/util/file/file_reader.h index 04724170..d44be1c1 100644 --- a/util/file/file_reader.h +++ b/util/file/file_reader.h @@ -15,7 +15,6 @@ #ifndef CRASHPAD_UTIL_FILE_FILE_READER_H_ #define CRASHPAD_UTIL_FILE_FILE_READER_H_ -#include #include #include "base/files/file_path.h" @@ -142,40 +141,6 @@ class FileReader : public FileReaderInterface { DISALLOW_COPY_AND_ASSIGN(FileReader); }; -//! \brief A file reader backed by a standard input/output `FILE*`. -//! -//! This class accepts an already-open `FILE*`. It is not responsible for -//! opening or closing this `FILE*`. Users of this class must ensure that the -//! `FILE*` is closed appropriately elsewhere. Objects of this class may be used -//! to read from `FILE*` objects not associated with filesystem-based files, -//! although special attention should be paid to the Seek() method, which may -//! not function on `FILE*` objects that do not refer to disk-based files. -//! -//! This class is expected to be used when other code is responsible for -//! opening `FILE*` objects and already provides `FILE*` objects. A good use -//! would be a WeakStdioFileReader for `stdin`. -class WeakStdioFileReader : public FileReaderInterface { - public: - explicit WeakStdioFileReader(FILE* file); - ~WeakStdioFileReader() override; - - // FileReaderInterface: - FileOperationResult Read(void* data, size_t size) override; - - // FileSeekerInterface: - - //! \copydoc FileReaderInterface::Seek() - //! - //! \note This method is only guaranteed to function on `FILE*` objects - //! referring to disk-based files. - FileOffset Seek(FileOffset offset, int whence) override; - - private: - FILE* file_; // weak - - DISALLOW_COPY_AND_ASSIGN(WeakStdioFileReader); -}; - } // namespace crashpad #endif // CRASHPAD_UTIL_FILE_FILE_READER_H_ diff --git a/util/file/file_writer.cc b/util/file/file_writer.cc index d22a1a3b..a8d92566 100644 --- a/util/file/file_writer.cc +++ b/util/file/file_writer.cc @@ -192,66 +192,4 @@ FileOffset FileWriter::Seek(FileOffset offset, int whence) { return weak_file_handle_file_writer_.Seek(offset, whence); } -WeakStdioFileWriter::WeakStdioFileWriter(FILE* file) - : file_(file) { -} - -WeakStdioFileWriter::~WeakStdioFileWriter() { -} - -bool WeakStdioFileWriter::Write(const void* data, size_t size) { - DCHECK(file_); - - size_t rv = fwrite(data, 1, size, file_); - if (rv != size) { - if (ferror(file_)) { - STDIO_PLOG(ERROR) << "fwrite"; - } else { - LOG(ERROR) << "fwrite: expected " << size << ", observed " << rv; - } - return false; - } - - return true; -} - -bool WeakStdioFileWriter::WriteIoVec(std::vector* iovecs) { - DCHECK(file_); - - if (iovecs->empty()) { - LOG(ERROR) << "WriteIoVec(): no iovecs"; - return false; - } - - for (const WritableIoVec& iov : *iovecs) { - if (!Write(iov.iov_base, iov.iov_len)) { - return false; - } - } - -#ifndef NDEBUG - // The interface says that |iovecs| is not sacred, so scramble it to make sure - // that nobody depends on it. - memset(&(*iovecs)[0], 0xa5, sizeof((*iovecs)[0]) * iovecs->size()); -#endif - - return true; -} - -FileOffset WeakStdioFileWriter::Seek(FileOffset offset, int whence) { - DCHECK(file_); - if (fseeko(file_, offset, whence) == -1) { - STDIO_PLOG(ERROR) << "fseeko"; - return -1; - } - - FileOffset new_offset = ftello(file_); - if (new_offset == -1) { - STDIO_PLOG(ERROR) << "ftello"; - return -1; - } - - return new_offset; -} - } // namespace crashpad diff --git a/util/file/file_writer.h b/util/file/file_writer.h index 007e5f3a..ed261ec5 100644 --- a/util/file/file_writer.h +++ b/util/file/file_writer.h @@ -167,41 +167,6 @@ class FileWriter : public FileWriterInterface { DISALLOW_COPY_AND_ASSIGN(FileWriter); }; -//! \brief A file writer backed by a standard input/output `FILE*`. -//! -//! This class accepts an already-open `FILE*`. It is not responsible for -//! opening or closing this `FILE*`. Users of this class must ensure that the -//! `FILE*` is closed appropriately elsewhere. Objects of this class may be used -//! to write to `FILE*` objects not associated with filesystem-based files, -//! although special attention should be paid to the Seek() method, which may -//! not function on `FILE*` objects that do not refer to disk-based files. -//! -//! This class is expected to be used when other code is responsible for -//! opening `FILE*` objects and already provides `FILE*` objects. A good use -//! would be a WeakStdioFileWriter for `stdout`. -class WeakStdioFileWriter : public FileWriterInterface { - public: - explicit WeakStdioFileWriter(FILE* file); - ~WeakStdioFileWriter() override; - - // FileWriterInterface: - bool Write(const void* data, size_t size) override; - bool WriteIoVec(std::vector* iovecs) override; - - // FileSeekerInterface: - - //! \copydoc FileWriterInterface::Seek() - //! - //! \note This method is only guaranteed to function on `FILE*` objects - //! referring to disk-based files. - FileOffset Seek(FileOffset offset, int whence) override; - - private: - FILE* file_; // weak - - DISALLOW_COPY_AND_ASSIGN(WeakStdioFileWriter); -}; - } // namespace crashpad #endif // CRASHPAD_UTIL_FILE_FILE_WRITER_H_