Stop using InputSource::unreadCh

Jay Berkenbilt
1 parent 1a888ee3
Showing 7 changed files with 27 additions and 21 deletions
ChangeLog
TODO
include/qpdf/InputSource.hh
libqpdf/FileInputSource.cc
libqpdf/QPDF.cc
libqpdf/QPDFTokenizer.cc
libtests/closed_file_input_source.cc
+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
@@ -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
 ======
@@ -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;
@@ -80,7 +80,7 @@ FileInputSource::findAndSkipNextEOL()
                     }
                     else if (! ((ch == '\r') || (ch == '\n')))
                     {
-                        this->unreadCh(ch);
+                        this->seek(-1, SEEK_CUR);
                         done = true;
                     }
                 }
@@ -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&lt;InputSource&gt; 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&lt;InputSource&gt; 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(),
@@ -855,7 +855,7 @@ QPDFTokenizer::readToken(PointerHolder&lt;InputSource&gt; input,
     if (unread_char)
     {
-	input->unreadCh(char_to_unread);
+	input->seek(-1, SEEK_CUR);
     }
     if (token.getType() != tt_eof)
@@ -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);