Stop using InputSource::unreadCh

7 files changed, 27 insertions, 21 deletions

diff --git a/ChangeLog b/ChangeLog
index 25ba511e..2071b467 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2020-10-18  Jay Berkenbilt  <ejb@ql.org>
+
+	* Note that InputSource::unreadCh is deprecated and will be
+	removed in qpdf 11. Use seek(-1, SEEK_CUR) instead. This is what
+	it has always effectively done with some input sources and some
+	operating systems which don't allow unreading other than the most
+	recently read character. InputSource::unreadCh is no longer used
+	internally within libqpdf.
+
 2020-10-16  Jay Berkenbilt  <ejb@ql.org>
 
 	* Accept pull request that improves how the Windows native crypto
diff --git a/TODO b/TODO
index 153074b9..2b0492d6 100644
--- a/TODO
+++ b/TODO
@@ -117,6 +117,10 @@ ABI Changes
 This is a list of changes to make next time there is an ABI change.
 Comments appear in the code prefixed by "ABI"
 
+* Consider removing InputSource::unreadCh. Maybe we can declare it
+  final and delete so it will be forced to be removed from derived
+  classes.
+
 C++-11
 ======
 
diff --git a/include/qpdf/InputSource.hh b/include/qpdf/InputSource.hh
index 8850828d..22e3abcd 100644
--- a/include/qpdf/InputSource.hh
+++ b/include/qpdf/InputSource.hh
@@ -85,9 +85,14 @@ class QPDF_DLL_CLASS InputSource
     virtual size_t read(char* buffer, size_t length) = 0;
 
     // Note: you can only unread the character you just read. The
-    // specific character is ignored by some implementations.
+    // specific character is ignored by some implementations. unreadCh
+    // will be removed from the API in qpdf 11.
     virtual void unreadCh(char ch) = 0;
 
+    // ABI: delete unreadCh, and direct people to seek backward by 1
+    // character instead.
+    // virtual void unreadCh(char ch) final = delete;
+
   protected:
     qpdf_offset_t last_offset;
 
diff --git a/libqpdf/FileInputSource.cc b/libqpdf/FileInputSource.cc
index cd8feda4..5ec6a170 100644
--- a/libqpdf/FileInputSource.cc
+++ b/libqpdf/FileInputSource.cc
@@ -80,7 +80,7 @@ FileInputSource::findAndSkipNextEOL()
                     }
                     else if (! ((ch == '\r') || (ch == '\n')))
                     {
-                        this->unreadCh(ch);
+                        this->seek(-1, SEEK_CUR);
                         done = true;
                     }
                 }
diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc
index 5aa2d98c..2ebf88b0 100644
--- a/libqpdf/QPDF.cc
+++ b/libqpdf/QPDF.cc
@@ -632,7 +632,7 @@ QPDF::read_xref(qpdf_offset_t xref_offset)
                 }
                 else
                 {
-                    this->m->file->unreadCh(ch);
+                    this->m->file->seek(-1, SEEK_CUR);
                     done = true;
                 }
             }
@@ -1604,7 +1604,7 @@ QPDF::readObject(PointerHolder<InputSource> input,
                             // start reading stream data in spite
                             // of not having seen a newline.
                             QTC::TC("qpdf", "QPDF stream with CR only");
-                            input->unreadCh(ch);
+                            input->seek(-1, SEEK_CUR);
                             warn(QPDFExc(
                                      qpdf_e_damaged_pdf,
                                      input->getName(),
@@ -1629,7 +1629,7 @@ QPDF::readObject(PointerHolder<InputSource> input,
                 else
                 {
                     QTC::TC("qpdf", "QPDF stream without newline");
-                    input->unreadCh(ch);
+                    input->seek(-1, SEEK_CUR);
                     warn(QPDFExc(qpdf_e_damaged_pdf, input->getName(),
                                  this->m->last_object_description,
                                  input->tell(),
diff --git a/libqpdf/QPDFTokenizer.cc b/libqpdf/QPDFTokenizer.cc
index 4217575c..4f2e2153 100644
--- a/libqpdf/QPDFTokenizer.cc
+++ b/libqpdf/QPDFTokenizer.cc
@@ -855,7 +855,7 @@ QPDFTokenizer::readToken(PointerHolder<InputSource> input,
 
     if (unread_char)
     {
-	input->unreadCh(char_to_unread);
+	input->seek(-1, SEEK_CUR);
     }
 
     if (token.getType() != tt_eof)
diff --git a/libtests/closed_file_input_source.cc b/libtests/closed_file_input_source.cc
index f3e4f2ac..2d58cf80 100644
--- a/libtests/closed_file_input_source.cc
+++ b/libtests/closed_file_input_source.cc
@@ -27,22 +27,10 @@ void do_tests(InputSource* is)
     check("tell after findAndSkipNextEOL", 522 == is->tell());
     char b[1];
     b[0] = '\0';
-#ifdef _WIN32
-    // Empirical evidence, and the passage of the rest of the qpdf
-    // test suite, suggest that this is working on Windows in the way
-    // that it needs to work. If this ifdef is made to be true on
-    // Windows, it passes with ClosedFileInputSource but not with
-    // FileInputSource, which doesn't make any sense since
-    // ClosedFileInputSource is calling FileInputSource to do its
-    // work.
-    is->seek(521, SEEK_SET);
-    is->read(b, 1);
-#else
-    is->unreadCh('\n');
-    check("read unread character", 1 == is->read(b, 1));
+    is->seek(-1, SEEK_CUR);
+    check("read previous character", 1 == is->read(b, 1));
     check("got character", '\n' == b[0]);
-#endif
-    check("last offset after read unread", 521 == is->getLastOffset());
+    check("last offset after read previous", 521 == is->getLastOffset());
     is->seek(0, SEEK_END);
     check("tell at end", 556 == is->tell());
     is->seek(-25, SEEK_END);