Change io::Reader and company to return ReadError::Eof instead of 0.

It's debatable if Eof should be considered an error or not.
But it is pretty clear it generally is a special response that
needs special handling, so easier to keep with the unexpected lot.

Also keeps better at higher abstraction levels, such as the line
reader.

12 files changed, 60 insertions, 65 deletions

diff --git a/src/csv.cc b/src/csv.cc
index c3b2562..4bc7804 100644
--- a/src/csv.cc
+++ b/src/csv.cc
@@ -30,10 +30,10 @@ class ReaderImpl : public Reader {
           continue;
         return line_;
       }
-      if (line.error().eof) {
+      if (line.error() == io::ReadError::Eof) {
         return {};
       }
-      return std::unexpected(line.error().io_error.value());
+      return std::unexpected(line.error());
     }
   }
 
diff --git a/src/decompress_lzma.cc b/src/decompress_lzma.cc
index 6ba2976..1946075 100644
--- a/src/decompress_lzma.cc
+++ b/src/decompress_lzma.cc
@@ -38,7 +38,7 @@ class XzReader : public io::Reader {
 
     if (!initialized_) {
       if (in_eof_ && buffer_->empty())
-        return 0;
+        return std::unexpected(io::ReadError::Eof);
 
       lzma_mt options;
       memset(&options, 0, sizeof(options));
@@ -89,8 +89,8 @@ class XzReader : public io::Reader {
       auto got = reader_->read(wptr, stream_.avail_in);
       if (got.has_value()) {
         buffer_->commit(got.value());
-        if (got.value() == 0)
-          in_eof_ = true;
+      } else if (got.error() == io::ReadError::Eof) {
+        in_eof_ = true;
       } else {
         return got.error();
       }
diff --git a/src/decompress_z.cc b/src/decompress_z.cc
index 491eff7..64f055c 100644
--- a/src/decompress_z.cc
+++ b/src/decompress_z.cc
@@ -39,7 +39,7 @@ class DecompressReader : public io::Reader {
 
     if (!initialized_) {
       if (in_eof_ && buffer_->empty())
-        return 0;
+        return std::unexpected(io::ReadError::Eof);
 
       stream_.zalloc = Z_NULL;
       stream_.zfree = Z_NULL;
@@ -89,8 +89,8 @@ class DecompressReader : public io::Reader {
       auto got = reader_->read(wptr, avail);
       if (got.has_value()) {
         buffer_->commit(got.value());
-        if (got.value() == 0)
-          in_eof_ = true;
+      } else if (got.error() == io::ReadError::Eof) {
+        in_eof_ = true;
       } else {
         return got.error();
       }
diff --git a/src/gen_ugc.cc b/src/gen_ugc.cc
index bc7c91b..2670803 100644
--- a/src/gen_ugc.cc
+++ b/src/gen_ugc.cc
@@ -151,6 +151,8 @@ std::string_view ioerr2str(io::ReadError error) {
       return "Fatal error";
     case io::ReadError::MaxTooSmall:
       return "Too small buffer";
+    case io::ReadError::Eof:
+      return "Unexpected end of file";
   }
   std::unreachable();
 }
diff --git a/src/io.cc b/src/io.cc
index 44970fe..99c0518 100644
--- a/src/io.cc
+++ b/src/io.cc
@@ -39,6 +39,8 @@ class BasicReader : public Reader {
         default:
           return std::unexpected(ReadError::Error);
       }
+    } else if (ret == 0 && max > 0) {
+      return std::unexpected(ReadError::Eof);
     }
     offset_ += ret;
     return ret;
@@ -60,7 +62,7 @@ class BasicReader : public Reader {
       return std::unexpected(ReadError::Error);
     }
     // Don't want skip to go past (cached) file end.
-    if (!size_.has_value() || ret > size_.value()) {
+    if (!size_.has_value() || ret >= size_.value()) {
       // When going past end, double check that it still is the end.
       off_t ret2 = lseek(fd_.get(), 0, SEEK_END);
       if (ret2 < 0) {
@@ -71,9 +73,11 @@ class BasicReader : public Reader {
         return std::unexpected(ReadError::Error);
       }
       size_ = ret2;
-      if (ret > ret2) {
+      if (ret >= ret2) {
         auto distance = ret2 - offset_;
         offset_ = ret2;
+        if (distance == 0 && max > 0)
+          return std::unexpected(ReadError::Eof);
         return distance;
       }
       // Seek back to where we should be
@@ -99,6 +103,8 @@ class MemoryReader : public Reader {
   [[nodiscard]]
   std::expected<size_t, ReadError> read(void* dst, size_t max) override {
     size_t avail = size_ - offset_;
+    if (avail == 0 && max > 0)
+      return std::unexpected(io::ReadError::Eof);
     size_t ret = std::min(max, avail);
     memcpy(dst, reinterpret_cast<char*>(ptr_) + offset_, ret);
     offset_ += ret;
@@ -147,14 +153,14 @@ class StringReader : public MemoryReader {
 
 std::expected<size_t, ReadError> Reader::repeat_read(void* dst, size_t max) {
   auto ret = read(dst, max);
-  if (!ret.has_value() || ret.value() == 0 || ret.value() == max)
+  if (!ret.has_value() || ret.value() == max)
     return ret;
 
   char* d = reinterpret_cast<char*>(dst);
   size_t offset = ret.value();
   while (true) {
     ret = read(d + offset, max - offset);
-    if (!ret.has_value() || ret.value() == 0)
+    if (!ret.has_value())
       break;
     offset += ret.value();
     if (offset == max)
@@ -165,13 +171,13 @@ std::expected<size_t, ReadError> Reader::repeat_read(void* dst, size_t max) {
 
 std::expected<size_t, ReadError> Reader::repeat_skip(size_t max) {
   auto ret = skip(max);
-  if (!ret.has_value() || ret.value() == 0 || ret.value() == max)
+  if (!ret.has_value() || ret.value() == max)
     return ret;
 
   size_t offset = ret.value();
   while (true) {
     ret = skip(max - offset);
-    if (!ret.has_value() || ret.value() == 0)
+    if (!ret.has_value())
       break;
     offset += ret.value();
     if (offset == max)
diff --git a/src/io.hh b/src/io.hh
index e93b72b..7c21028 100644
--- a/src/io.hh
+++ b/src/io.hh
@@ -10,6 +10,7 @@ namespace io {
 
 enum class ReadError {
   Error,
+  Eof,
   InvalidData,  // invalid data read (not used by raw file)
   MaxTooSmall,  // max argument needs to be bigger (not used by raw file)
 };
diff --git a/src/java_uescape.cc b/src/java_uescape.cc
index 925b050..3951b5f 100644
--- a/src/java_uescape.cc
+++ b/src/java_uescape.cc
@@ -46,9 +46,8 @@ class UnicodeEscapeReader {
     if (fill_ == 0) {
       // Optimize for the case when there are no unicode escapes.
       auto ret = reader_->read(dst, max);
-      if (!ret.has_value() || ret.value() == 0) {
+      if (!ret.has_value())
         return ret;
-      }
 
       eof = false;
       state.rstart = reinterpret_cast<T*>(dst);
@@ -63,10 +62,14 @@ class UnicodeEscapeReader {
       auto ret = reader_->read(buffer_.get() + fill_,
                                (kBufferSize - fill_) * sizeof(T));
       if (!ret.has_value()) {
-        return ret;
+        if (ret.error() != io::ReadError::Eof) {
+          return ret;
+        }
+        eof = true;
+      } else {
+        eof = false;
+        fill_ += ret.value() / sizeof(T);
       }
-      eof = ret.value() == 0;
-      fill_ += ret.value() / sizeof(T);
 
       state.rstart = buffer_.get();
       state.rend = buffer_.get() + fill_;
@@ -228,7 +231,7 @@ class UnicodeEscapeReader {
       fill_ += avail;
     }
 
-    // We can't return zero, that is treated as EOF, we need to read more.
+    // We shouldn't return zero, we should read more if we can.
     if (state.wptr == state.wstart) {
       if (fill_ > 0) {
         return read(state.wstart, (state.wend - state.wstart) * sizeof(T));
diff --git a/src/line.cc b/src/line.cc
index 8ee7134..23370fc 100644
--- a/src/line.cc
+++ b/src/line.cc
@@ -29,7 +29,7 @@ class ReaderImpl : public Reader {
         search_(rptr_),
         end_(buffer_.get() + check::add(max_len, static_cast<size_t>(2))) {}
 
-  [[nodiscard]] std::expected<std::string_view, ReadError> read() override {
+  [[nodiscard]] std::expected<std::string_view, io::ReadError> read() override {
     while (true) {
       search_ = std::find_first_of(search_, wptr_, kLineTerminators,
                                    kLineTerminators + 2);
@@ -45,12 +45,11 @@ class ReaderImpl : public Reader {
           if (search_ + 1 == wptr_) {
             make_space_if_needed();
             auto got = fill();
-            if (got.has_value()) {
-              if (got.value() == 0) {
+            if (!got.has_value()) {
+              if (got.error() == io::ReadError::Eof) {
                 return line(search_ - rptr_, 1);
               }
-            } else {
-              return std::unexpected(ReadError(got.error()));
+              return std::unexpected(got.error());
             }
           }
           if (search_[1] == '\n') {
@@ -67,15 +66,11 @@ class ReaderImpl : public Reader {
 
       make_space_if_needed();
       auto got = fill();
-      if (got.has_value()) {
-        if (got.value() == 0) {
-          if (rptr_ == wptr_) {
-            return std::unexpected(ReadError());
-          }
+      if (!got.has_value()) {
+        if (got.error() == io::ReadError::Eof && rptr_ != wptr_) {
           return line(wptr_ - rptr_, 0);
         }
-      } else {
-        return std::unexpected(ReadError(got.error()));
+        return std::unexpected(got.error());
       }
     }
   }
@@ -124,10 +119,6 @@ class ReaderImpl : public Reader {
 
 }  // namespace
 
-ReadError::ReadError() : eof(true) {}
-
-ReadError::ReadError(io::ReadError error) : eof(false), io_error(error) {}
-
 std::unique_ptr<Reader> open(std::unique_ptr<io::Reader> reader,
                              size_t max_len) {
   return std::make_unique<ReaderImpl>(std::move(reader), max_len);
diff --git a/src/line.hh b/src/line.hh
index 5ce42dd..a8eeea8 100644
--- a/src/line.hh
+++ b/src/line.hh
@@ -11,20 +11,13 @@
 
 namespace line {
 
-struct ReadError {
-  bool eof;
-  std::optional<io::ReadError> io_error;
-
-  ReadError();
-  explicit ReadError(io::ReadError error);
-};
-
 class Reader {
  public:
   virtual ~Reader() = default;
 
   // Returned view is only valid until next call to read.
-  [[nodiscard]] virtual std::expected<std::string_view, ReadError> read() = 0;
+  [[nodiscard]]
+  virtual std::expected<std::string_view, io::ReadError> read() = 0;
   // Starts at zero. Returns next line.
   // So, before first read it is zero, after first read it is one.
   [[nodiscard]] virtual uint64_t number() const = 0;
diff --git a/src/uio.cc b/src/uio.cc
index 9fc22c1..da943a2 100644
--- a/src/uio.cc
+++ b/src/uio.cc
@@ -50,6 +50,11 @@ class UnicodeReader : public io::Reader {
             return std::unexpected(io::ReadError::InvalidData);
           break;
         case u::ReadError::End:
+          if (in_eof_) {
+            // Only return eof if we have no bytes to output.
+            if (u_buffer_wptr_ == u_buffer_)
+              return std::unexpected(io::ReadError::Eof);
+          }
           break;
         case u::ReadError::Incomplete:
           if (in_eof_) {
@@ -106,8 +111,8 @@ class UnicodeReader : public io::Reader {
       auto got = in_->read(wptr, in_avail_);
       if (got.has_value()) {
         byte_buffer_->commit(got.value());
-        if (got.value() == 0)
-          in_eof_ = true;
+      } else if (got.error() == io::ReadError::Eof) {
+        in_eof_ = true;
       } else {
         return got.error();
       }
diff --git a/src/uline.cc b/src/uline.cc
index 7d4c7c7..358f36c 100644
--- a/src/uline.cc
+++ b/src/uline.cc
@@ -30,7 +30,7 @@ class UnicodeReader {
         end_(buffer_.get() + check::add(max_len, static_cast<size_t>(2))) {}
 
   [[nodiscard]]
-  std::expected<std::basic_string_view<T>, line::ReadError> read() {
+  std::expected<std::basic_string_view<T>, io::ReadError> read() {
     while (true) {
       search_ = std::find_first_of(search_, wptr_, line_terminators_.begin(),
                                    line_terminators_.end());
@@ -46,12 +46,11 @@ class UnicodeReader {
           if (search_ + 1 == wptr_) {
             make_space_if_needed();
             auto got = fill();
-            if (got.has_value()) {
-              if (got.value() == 0) {
+            if (!got.has_value()) {
+              if (got.error() == io::ReadError::Eof) {
                 return line(search_ - rptr_, 1);
               }
-            } else {
-              return std::unexpected(line::ReadError(got.error()));
+              return std::unexpected(got.error());
             }
           }
           if (search_[1] == line_terminators_[1]) {
@@ -68,15 +67,11 @@ class UnicodeReader {
 
       make_space_if_needed();
       auto got = fill();
-      if (got.has_value()) {
-        if (got.value() == 0) {
-          if (rptr_ == wptr_) {
-            return std::unexpected(line::ReadError());
-          }
+      if (!got.has_value()) {
+        if (got.error() == io::ReadError::Eof && rptr_ != wptr_) {
           return line(wptr_ - rptr_, 0);
         }
-      } else {
-        return std::unexpected(line::ReadError(got.error()));
+        return std::unexpected(got.error());
       }
     }
   }
@@ -138,7 +133,7 @@ class ReaderImpl : public UnicodeReader<char, u8::Reader>,
                                         {'\r', '\n'}) {}
 
   [[nodiscard]]
-  std::expected<std::string_view, ::line::ReadError> read() override {
+  std::expected<std::string_view, io::ReadError> read() override {
     return UnicodeReader<char, u8::Reader>::read();
   }
 
@@ -170,7 +165,7 @@ class ReaderImpl : public UnicodeReader<char16_t, u16::Reader>,
                                              {u'\r', u'\n'}) {}
 
   [[nodiscard]]
-  std::expected<std::u16string_view, ::line::ReadError> read() override {
+  std::expected<std::u16string_view, io::ReadError> read() override {
     return UnicodeReader<char16_t, u16::Reader>::read();
   }
 
diff --git a/src/uline.hh b/src/uline.hh
index 28e2936..e6ad9c1 100644
--- a/src/uline.hh
+++ b/src/uline.hh
@@ -1,8 +1,7 @@
 #ifndef ULINE_HH
 #define ULINE_HH
 
-#include "line.hh"  // IWYU pragma: export
-#include "uio.hh"   // IWYU pragma: export
+#include "uio.hh"  // IWYU pragma: export
 
 #include <cstddef>
 #include <expected>
@@ -17,7 +16,7 @@ class Reader {
 
   // Returned view is only valid until next call to read.
   [[nodiscard]]
-  virtual std::expected<std::string_view, ::line::ReadError> read() = 0;
+  virtual std::expected<std::string_view, io::ReadError> read() = 0;
 
   // Starts at zero. Returns next line.
   // So, before first read it is zero, after first read it is one.
@@ -43,7 +42,7 @@ class Reader {
 
   // Returned view is only valid until next call to read.
   [[nodiscard]]
-  virtual std::expected<std::u16string_view, ::line::ReadError> read() = 0;
+  virtual std::expected<std::u16string_view, io::ReadError> read() = 0;
 
   // Starts at zero. Returns next line.
   // So, before first read it is zero, after first read it is one.