Some improvements to util/file_input_stream.{cc|h}

 * Add check for `data` being nullptr and unit test for it
 * Default initialise class members
 * Use a std::vector<uint8_t> instead of a heap allocated array; remove buffer_size_ as no longer needed

PiperOrigin-RevId: 502824318

4 files changed, 31 insertions, 35 deletions

diff --git a/cc/util/BUILD.bazel b/cc/util/BUILD.bazel
index de77a8000..a22486242 100644
--- a/cc/util/BUILD.bazel
+++ b/cc/util/BUILD.bazel
@@ -167,7 +167,6 @@ cc_library(
         ":status",
         ":statusor",
         "//:input_stream",
-        "@com_google_absl//absl/memory",
         "@com_google_absl//absl/status",
     ],
 )
diff --git a/cc/util/CMakeLists.txt b/cc/util/CMakeLists.txt
index 083663385..ed07b9355 100644
--- a/cc/util/CMakeLists.txt
+++ b/cc/util/CMakeLists.txt
@@ -89,7 +89,6 @@ tink_cc_library(
     tink::util::errors
     tink::util::status
     tink::util::statusor
-    absl::memory
     absl::status
     tink::core::input_stream
 )
diff --git a/cc/util/file_input_stream.cc b/cc/util/file_input_stream.cc
index 3903f2003..695651afa 100644
--- a/cc/util/file_input_stream.cc
+++ b/cc/util/file_input_stream.cc
@@ -19,9 +19,7 @@
 #include <unistd.h>
 #include <algorithm>
 
-#include "absl/memory/memory.h"
 #include "absl/status/status.h"
-#include "tink/input_stream.h"
 #include "tink/util/errors.h"
 #include "tink/util/status.h"
 #include "tink/util/statusor.h"
@@ -29,9 +27,10 @@
 namespace crypto {
 namespace tink {
 namespace util {
-
 namespace {
 
+constexpr int kDefaultBufferSize = 128 * 1024;
+
 // Attempts to close file descriptor fd, while ignoring EINTR.
 // (code borrowed from ZeroCopy-streams)
 int close_ignoring_eintr(int fd) {
@@ -42,7 +41,6 @@ int close_ignoring_eintr(int fd) {
   return result;
 }
 
-
 // Attempts to read 'count' bytes of data data from file descriptor fd
 // to 'buf' while ignoring EINTR.
 int read_ignoring_eintr(int fd, void *buf, size_t count) {
@@ -55,29 +53,27 @@ int read_ignoring_eintr(int fd, void *buf, size_t count) {
 
 }  // anonymous namespace
 
-FileInputStream::FileInputStream(int file_descriptor, int buffer_size) :
-    buffer_size_(buffer_size > 0 ? buffer_size : 128 * 1024) {  // 128 KB
-  fd_ = file_descriptor;
-  count_in_buffer_ = 0;
-  count_backedup_ = 0;
-  position_ = 0;
-  buffer_ = absl::make_unique<uint8_t[]>(buffer_size_);
-  buffer_offset_ = 0;
-  status_ = util::OkStatus();
-}
+FileInputStream::FileInputStream(int file_descriptor, int buffer_size)
+    : status_(util::OkStatus()),
+      fd_(file_descriptor),
+      buffer_(buffer_size > 0 ? buffer_size : kDefaultBufferSize) {}
 
-crypto::tink::util::StatusOr<int> FileInputStream::Next(const void** data) {
+util::StatusOr<int> FileInputStream::Next(const void** data) {
+  if (data == nullptr) {
+    return util::Status(absl::StatusCode::kInvalidArgument,
+                        "Data pointer must not be nullptr");
+  }
   if (!status_.ok()) return status_;
   if (count_backedup_ > 0) {  // Return the backed-up bytes.
     buffer_offset_ = buffer_offset_ + (count_in_buffer_ - count_backedup_);
     count_in_buffer_ = count_backedup_;
     count_backedup_ = 0;
-    *data = buffer_.get() + buffer_offset_;
+    *data = buffer_.data() + buffer_offset_;
     position_ = position_ + count_in_buffer_;
     return count_in_buffer_;
   }
   // Read new bytes to buffer_.
-  int read_result = read_ignoring_eintr(fd_, buffer_.get(), buffer_size_);
+  int read_result = read_ignoring_eintr(fd_, buffer_.data(), buffer_.size());
   if (read_result <= 0) {  // EOF or an I/O error.
     if (read_result == 0) {
       status_ = Status(absl::StatusCode::kOutOfRange, "EOF");
@@ -91,7 +87,7 @@ crypto::tink::util::StatusOr<int> FileInputStream::Next(const void** data) {
   count_backedup_ = 0;
   count_in_buffer_ = read_result;
   position_ = position_ + count_in_buffer_;
-  *data = buffer_.get();
+  *data = buffer_.data();
   return count_in_buffer_;
 }
 
@@ -102,13 +98,9 @@ void FileInputStream::BackUp(int count) {
   position_ = position_ - actual_count;
 }
 
-FileInputStream::~FileInputStream() {
-  close_ignoring_eintr(fd_);
-}
+FileInputStream::~FileInputStream() { close_ignoring_eintr(fd_); }
 
-int64_t FileInputStream::Position() const {
-  return position_;
-}
+int64_t FileInputStream::Position() const { return position_; }
 
 }  // namespace util
 }  // namespace tink
diff --git a/cc/util/file_input_stream.h b/cc/util/file_input_stream.h
index 5c816b06f..bec4d23c6 100644
--- a/cc/util/file_input_stream.h
+++ b/cc/util/file_input_stream.h
@@ -17,7 +17,9 @@
 #ifndef TINK_UTIL_FILE_INPUT_STREAM_H_
 #define TINK_UTIL_FILE_INPUT_STREAM_H_
 
+#include <cstdint>
 #include <memory>
+#include <vector>
 
 #include "tink/input_stream.h"
 #include "tink/util/status.h"
@@ -31,8 +33,8 @@ namespace util {
 class FileInputStream : public crypto::tink::InputStream {
  public:
   // Constructs an InputStream that will read from the file specified
-  // via 'file_descriptor', using a buffer of the specified size, if any
-  // (if no legal 'buffer_size' is given, a reasonable default will be used).
+  // via `file_descriptor`, using a buffer of the specified size, if any
+  // (if no legal `buffer_size` is given, a reasonable default will be used).
   // Takes the ownership of the file, and will close it upon destruction.
   explicit FileInputStream(int file_descriptor, int buffer_size = -1);
 
@@ -45,16 +47,20 @@ class FileInputStream : public crypto::tink::InputStream {
   int64_t Position() const override;
 
  private:
-  util::Status status_;
+  // Status of the stream.
+  util::Status status_ = util::OkStatus();
   int fd_;
-  std::unique_ptr<uint8_t[]> buffer_;
-  const int buffer_size_;
-  int64_t position_;     // current position in the file (from the beginning)
+  std::vector<uint8_t> buffer_;
 
+  // Current position in the stream (from the beginning).
+  int64_t position_ = 0;
   // Counters that describe the state of the data in buffer_.
-  int count_in_buffer_;  // # of bytes available in buffer_
-  int count_backedup_;   // # of bytes available in buffer_ that were backed up
-  int buffer_offset_;    // offset at which the returned bytes start in buffer_
+  // # of bytes available in buffer_.
+  int count_in_buffer_ = 0;
+  // # of bytes available in buffer_ that were backed up.
+  int count_backedup_ = 0;
+  // offset at which the returned bytes start in buffer_.
+  int buffer_offset_ = 0;
 };
 
 }  // namespace util