From 89ca2fbba75120b827df555f947d508b54d5be47 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Wed, 18 Feb 2015 18:22:39 -0500 Subject: [PATCH] Add FileReaderInterface::ReadExactly() and FileSeekerInterface::SeekSet(). These methods perform common error checking. TEST=util_test StringFile.* R=rsesek@chromium.org Review URL: https://codereview.chromium.org/913223008 --- util/file/file_reader.cc | 14 +++++++++++++ util/file/file_reader.h | 11 ++++++++++- util/file/file_seeker.cc | 37 +++++++++++++++++++++++++++++++++++ util/file/file_seeker.h | 15 ++++++++++++++ util/file/file_writer.h | 2 +- util/file/string_file_test.cc | 27 +++++++++++++++++++++++++ util/util.gyp | 1 + 7 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 util/file/file_seeker.cc diff --git a/util/file/file_reader.cc b/util/file/file_reader.cc index b6fd0173..c983f4f6 100644 --- a/util/file/file_reader.cc +++ b/util/file/file_reader.cc @@ -19,6 +19,20 @@ namespace crashpad { +bool FileReaderInterface::ReadExactly(void* data, size_t size) { + ssize_t expect = base::checked_cast(size); + ssize_t rv = Read(data, size); + if (rv < 0) { + // Read() will have logged its own error. + return false; + } else if (rv != expect) { + LOG(ERROR) << "ReadExactly(): expected " << expect << ", observed " << rv; + return false; + } + + return true; +} + WeakFileHandleFileReader::WeakFileHandleFileReader(FileHandle file_handle) : file_handle_(file_handle) { } diff --git a/util/file/file_reader.h b/util/file/file_reader.h index 825a38e4..2d699cb4 100644 --- a/util/file/file_reader.h +++ b/util/file/file_reader.h @@ -26,7 +26,7 @@ namespace crashpad { //! \brief An interface to read to files and other file-like objects with //! semantics matching the underlying platform (POSIX or Windows). -class FileReaderInterface : public FileSeekerInterface { +class FileReaderInterface : public virtual FileSeekerInterface { public: //! \brief Wraps ReadFile(), or provides an implementation with identical //! semantics. @@ -36,6 +36,15 @@ class FileReaderInterface : public FileSeekerInterface { //! `-1` if the operation failed, with an error message logged. virtual ssize_t Read(void* data, size_t size) = 0; + //! \brief Wraps Read(), ensuring that the read succeeded and exactly \a size + //! bytes were read. + //! + //! Semantically, this behaves as LoggingReadFile(). + //! + //! \return `true` if the operation succeeded, `false` if it failed, with an + //! error message logged. Short reads are treated as failures. + bool ReadExactly(void* data, size_t size); + protected: ~FileReaderInterface() {} }; diff --git a/util/file/file_seeker.cc b/util/file/file_seeker.cc new file mode 100644 index 00000000..a38e2f91 --- /dev/null +++ b/util/file/file_seeker.cc @@ -0,0 +1,37 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// 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 "util/file/file_seeker.h" + +#include "base/logging.h" + +namespace crashpad { + +FileOffset FileSeekerInterface::SeekGet() { + return Seek(0, SEEK_CUR); +} + +bool FileSeekerInterface::SeekSet(FileOffset offset) { + FileOffset rv = Seek(offset, SEEK_SET); + if (rv < 0) { + // Seek() will have logged its own error. + return false; + } else if (rv != offset) { + LOG(ERROR) << "SeekSet(): expected " << offset << ", observed " << rv; + return false; + } + return true; +} + +} // namespace crashpad diff --git a/util/file/file_seeker.h b/util/file/file_seeker.h index f6d8b464..1b32e104 100644 --- a/util/file/file_seeker.h +++ b/util/file/file_seeker.h @@ -30,6 +30,21 @@ class FileSeekerInterface { //! with an error message logged. virtual FileOffset Seek(FileOffset offset, int whence) = 0; + //! \brief Wraps Seek(), using `SEEK_CUR` to obtain the file’s current + //! position. + //! + //! \return The file’s current position on success. `-1` on failure, with an + //! error message logged. + FileOffset SeekGet(); + + //! \brief Wraps Seek(), using `SEEK_SET`, ensuring that the seek succeeded + //! and the file is positioned as desired. + //! + //! \return `true` if the operation succeeded, `false` if it failed, with an + //! error message logged. A failure to reposition the file as desired is + //! treated as a failure. + bool SeekSet(FileOffset offset); + protected: ~FileSeekerInterface() {} }; diff --git a/util/file/file_writer.h b/util/file/file_writer.h index 2b0c97ed..fd88ce08 100644 --- a/util/file/file_writer.h +++ b/util/file/file_writer.h @@ -42,7 +42,7 @@ struct WritableIoVec { //! \brief An interface to write to files and other file-like objects with //! semantics matching the underlying platform (POSIX or Windows). -class FileWriterInterface : public FileSeekerInterface { +class FileWriterInterface : public virtual FileSeekerInterface { public: //! \brief Wraps LoggingWriteFile(), or provides an implementation with //! identical semantics. diff --git a/util/file/string_file_test.cc b/util/file/string_file_test.cc index 93301f1f..c585610b 100644 --- a/util/file/string_file_test.cc +++ b/util/file/string_file_test.cc @@ -117,6 +117,17 @@ TEST(StringFile, SetString) { EXPECT_EQ(kString3, string_file.string()); } +TEST(StringFile, ReadExactly) { + StringFile string_file; + string_file.SetString("1234567"); + char buf[4] = "***"; + EXPECT_TRUE(string_file.ReadExactly(buf, 3)); + EXPECT_STREQ("123", buf); + EXPECT_TRUE(string_file.ReadExactly(buf, 3)); + EXPECT_STREQ("456", buf); + EXPECT_FALSE(string_file.ReadExactly(buf, 3)); +} + TEST(StringFile, Reset) { StringFile string_file; @@ -465,6 +476,22 @@ TEST(StringFile, SeekInvalid) { EXPECT_LT(string_file.Seek(kMaxOffset, SEEK_CUR), 0); } +TEST(StringFile, SeekSet) { + StringFile string_file; + EXPECT_TRUE(string_file.SeekSet(1)); + EXPECT_EQ(1, string_file.Seek(0, SEEK_CUR)); + EXPECT_TRUE(string_file.SeekSet(0)); + EXPECT_EQ(0, string_file.Seek(0, SEEK_CUR)); + EXPECT_TRUE(string_file.SeekSet(10)); + EXPECT_EQ(10, string_file.Seek(0, SEEK_CUR)); + EXPECT_FALSE(string_file.SeekSet(-1)); + EXPECT_EQ(10, string_file.Seek(0, SEEK_CUR)); + EXPECT_FALSE(string_file.SeekSet(std::numeric_limits::min())); + EXPECT_EQ(10, string_file.Seek(0, SEEK_CUR)); + EXPECT_FALSE(string_file.SeekSet(std::numeric_limits::min())); + EXPECT_EQ(10, string_file.Seek(0, SEEK_CUR)); +} + } // namespace } // namespace test } // namespace crashpad diff --git a/util/util.gyp b/util/util.gyp index 95ba729d..ddb4695d 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -32,6 +32,7 @@ 'file/file_io_win.cc', 'file/file_reader.cc', 'file/file_reader.h', + 'file/file_seeker.cc', 'file/file_seeker.h', 'file/file_writer.cc', 'file/file_writer.h',