Refactor QUtil::read_lines_from_file

This commit adds the preserve_eol flags but doesn't implement EOL
preservation yet.

include/qpdf/QUtil.hh

@@ -29,6 +29,7 @@
 #include <list>
 #include <vector>
 #include <stdexcept>
+#include <functional>
 #include <stdio.h>
 #include <time.h>
 
@@ -90,8 +91,9 @@ namespace QUtil
     QPDF_DLL
     int os_wrapper(std::string const& description, int status);
 
-    // If the open fails, throws std::runtime_error.  Otherwise, the
+    // If the open fails, throws std::runtime_error. Otherwise, the
-    // FILE* is returned.
+    // FILE* is returned. The filename should be UTF-8 encoded, even
+    // on Windows. It will be converted as needed on Windows.
     QPDF_DLL
     FILE* safe_fopen(char const* filename, char const* mode);
 
@@ -308,10 +310,29 @@ namespace QUtil
     QPDF_DLL
     RandomDataProvider* getRandomDataProvider();
 
+    // Filename is UTF-8 encoded, even on Windows, as described in the
+    // comments for safe_fopen.
     QPDF_DLL
     std::list<std::string> read_lines_from_file(char const* filename);
+    // ABI: make preserve_eol an optional arg and remove single-arg version
+    QPDF_DLL
+    std::list<std::string> read_lines_from_file(
+        char const* filename, bool preserve_eol);
     QPDF_DLL
     std::list<std::string> read_lines_from_file(std::istream&);
+    // ABI: make preserve_eol an optional arg and remove single-arg version
+    QPDF_DLL
+    std::list<std::string> read_lines_from_file(
+        std::istream&, bool preserve_eol);
+    QPDF_DLL
+    std::list<std::string> read_lines_from_file(
+        FILE*, bool preserve_eol = false);
+    QPDF_DLL
+    void read_lines_from_file(
+        std::function<bool(char&)> next_char,
+        std::list<std::string>& lines,
+        bool preserve_eol = false);
+
     QPDF_DLL
     void read_file_into_memory(
         char const* filename, PointerHolder<char>& file_buf, size_t& size);

libqpdf/QUtil.cc

@@ -234,6 +234,23 @@ static unsigned short mac_roman_to_unicode[] = {
     0x02c7,    // 0xff
 };
 
+class FileCloser
+{
+  public:
+    FileCloser(FILE* f) :
+        f(f)
+    {
+    }
+
+    ~FileCloser()
+    {
+        fclose(f);
+    }
+
+  private:
+    FILE* f;
+};
+
 template <typename T>
 static
 std::string
@@ -987,19 +1004,6 @@ QUtil::is_number(char const* p)
     return found_digit;
 }
 
-std::list<std::string>
-QUtil::read_lines_from_file(char const* filename)
-{
-    std::ifstream in(filename, std::ios_base::binary);
-    if (! in.is_open())
-    {
-        throw_system_error(std::string("open ") + filename);
-    }
-    std::list<std::string> lines = read_lines_from_file(in);
-    in.close();
-    return lines;
-}
-
 void
 QUtil::read_file_into_memory(
     char const* filename,
@@ -1039,19 +1043,70 @@ QUtil::read_file_into_memory(
     fclose(f);
 }
 
+std::list<std::string>
+QUtil::read_lines_from_file(char const* filename)
+{
+    // ABI: remove this method
+    std::list<std::string> lines;
+    FILE* f = safe_fopen(filename, "rb");
+    FileCloser fc(f);
+    auto next_char = [&f](char& ch) { return (fread(&ch, 1, 1, f) > 0); };
+    read_lines_from_file(next_char, lines, false);
+    return lines;
+}
+
+std::list<std::string>
+QUtil::read_lines_from_file(char const* filename, bool preserve_eol)
+{
+    std::list<std::string> lines;
+    FILE* f = safe_fopen(filename, "rb");
+    FileCloser fc(f);
+    auto next_char = [&f](char& ch) { return (fread(&ch, 1, 1, f) > 0); };
+    read_lines_from_file(next_char, lines, preserve_eol);
+    return lines;
+}
+
 std::list<std::string>
 QUtil::read_lines_from_file(std::istream& in)
 {
-    std::list<std::string> result;
+    // ABI: remove this method
-    std::string* buf = 0;
+    std::list<std::string> lines;
+    auto next_char = [&in](char& ch) { return (in.get(ch)) ? true: false; };
+    read_lines_from_file(next_char, lines, false);
+    return lines;
+}
 
+std::list<std::string>
+QUtil::read_lines_from_file(std::istream& in, bool preserve_eol)
+{
+    std::list<std::string> lines;
+    auto next_char = [&in](char& ch) { return (in.get(ch)) ? true: false; };
+    read_lines_from_file(next_char, lines, preserve_eol);
+    return lines;
+}
+
+std::list<std::string>
+QUtil::read_lines_from_file(FILE* f, bool preserve_eol)
+{
+    std::list<std::string> lines;
+    auto next_char = [&f](char& ch) { return (fread(&ch, 1, 1, f) > 0); };
+    read_lines_from_file(next_char, lines, preserve_eol);
+    return lines;
+}
+
+void
+QUtil::read_lines_from_file(std::function<bool(char&)> next_char,
+                            std::list<std::string>& lines,
+                            bool preserve_eol)
+{
+    std::string* buf = 0;
     char c;
-    while (in.get(c))
+    while (next_char(c))
     {
 	if (buf == 0)
 	{
-	    result.push_back("");
+	    lines.push_back("");
-	    buf = &(result.back());
+	    buf = &(lines.back());
 	    buf->reserve(80);
 	}
 
@@ -1074,8 +1129,6 @@ QUtil::read_lines_from_file(std::istream& in)
 	    buf->append(1, c);
 	}
     }
-
-    return result;
 }
 
 int

libtests/qutil.cc

@@ -9,6 +9,7 @@
 #include <string.h>
 #include <limits.h>
 #include <assert.h>
+#include <fstream>
 
 #ifdef _WIN32
 # include <io.h>
@@ -408,6 +409,15 @@ void read_from_file_test()
     {
         std::cout << *iter << std::endl;
     }
+    // Test the other versions and make sure we get the same results
+    {
+        std::ifstream infs("other-file", std::ios_base::binary);
+        assert(QUtil::read_lines_from_file(infs) == lines);
+        FILE* fp = QUtil::safe_fopen("other-file", "rb");
+        assert(QUtil::read_lines_from_file(fp) == lines);
+        fclose(fp);
+    }
+
     PointerHolder<char> buf;
     size_t size = 0;
     QUtil::read_file_into_memory("other-file", buf, size);