Changed parser semantics around skipping.

Prior to this change:

  parse(buf, len) -> len + N

...would indicate that the next N bytes of the input are not
needed, *and* would advance the decoding position by this
much.

After this change:

  parse(buf, len) -> len + N
  parse(NULL, N) -> N

...can be used to achieve the same thing.  But skipping the
N bytes is not explicitly performed by the user.  A user that
doesn't want/need to skip can just say:

  parsed = parse(buf, len);

  if (parsed < len) {
    // Handle suspend, advance stream by "parsed".
  } else {
    // Stream was advanced by "len" (even if parsed > len).
  }

Updated unit tests to test this new behavior, and refactored
test utility code a bit to support it.

1 files changed, 102 insertions, 33 deletions

diff --git a/tests/test_util.h b/tests/test_util.h
index d9e8d25..6408557 100644
--- a/tests/test_util.h
+++ b/tests/test_util.h
@@ -8,59 +8,90 @@
 #include <stdio.h>
 #include <math.h>
 #include "tests/upb_test.h"
+#include "upb/env.h"
 #include "upb/sink.h"
 
 upb::BufferHandle global_handle;
 
-// Puts a region of the given buffer [start, end) into the given sink (which
-// probably represents a parser.  Can gracefully handle the case where the
-// parser returns a "parsed" length that is less or greater than the input
-// buffer length, and tracks the overall parse offset in *ofs.
-//
-// Pass verbose=true to print detailed diagnostics to stderr.
-bool parse_buffer(upb::BytesSink* sink, void* subc, const char* buf,
-                  size_t start, size_t end, size_t* ofs,
-                  upb::Status* status, bool verbose) {
-  start = UPB_MAX(start, *ofs);
+class VerboseParserEnvironment {
+ public:
+  VerboseParserEnvironment(bool verbose) : verbose_(verbose) {
+    env_.ReportErrorsTo(&status_);
+  }
+
+  void Reset(const char *buf, size_t len, bool may_skip) {
+    buf_ = buf;
+    len_ = len;
+    ofs_ = 0;
+    skip_until_ = may_skip ? 0 : -1;
+    status_.Clear();
+  }
+
+  bool Start() {
+    return sink_->Start(len_, &subc_);
+  }
+
+  bool End() {
+    return sink_->End();
+  }
+
+  // Puts a region of the given buffer [start, end) into the given sink (which
+  // probably represents a parser.  Can gracefully handle the case where the
+  // parser returns a "parsed" length that is less or greater than the input
+  // buffer length, and tracks the overall parse offset in *ofs.
+  //
+  // Pass verbose=true to print detailed diagnostics to stderr.
+  bool ParseBuffer(int bytes) {
+    if (bytes < 0) {
+      bytes = len_ - ofs_;
+    }
 
-  if (start <= end) {
-    size_t len = end - start;
+    ASSERT((size_t)bytes <= (len_ - ofs_));
 
     // Copy buffer into a separate, temporary buffer.
     // This is necessary to verify that the parser is not erroneously
     // reading outside the specified bounds.
-    char *buf2 = (char*)malloc(len);
-    assert(buf2);
-    memcpy(buf2, buf + start, len);
+    char *buf2 = NULL;
 
-    if (verbose) {
+    if ((int)(ofs_ + bytes) > skip_until_) {
+      buf2 = (char*)malloc(bytes);
+      assert(buf2);
+      memcpy(buf2, buf_ + ofs_, bytes);
+    }
+
+    if (buf2 == NULL && bytes == 0) {
+      // Decoders dont' support buf=NULL, bytes=0.
+      return true;
+    }
+
+    if (verbose_) {
       fprintf(stderr, "Calling parse(%u) for bytes %u-%u of the input\n",
-              (unsigned)len, (unsigned)start, (unsigned)end);
+              (unsigned)bytes, (unsigned)ofs_, (unsigned)(ofs_ + bytes));
     }
 
-    size_t parsed = sink->PutBuffer(subc, buf2, len, &global_handle);
+    int parsed = sink_->PutBuffer(subc_, buf2, bytes, &global_handle);
     free(buf2);
 
-    if (verbose) {
-      if (parsed == len) {
+    if (verbose_) {
+      if (parsed == bytes) {
         fprintf(stderr,
                 "parse(%u) = %u, complete byte count indicates success\n",
-                (unsigned)len, (unsigned)len);
-      } else if (parsed > len) {
+                (unsigned)bytes, (unsigned)bytes);
+      } else if (parsed > bytes) {
         fprintf(stderr,
-                "parse(%u) = %u, long byte count indicates success and skip"
+                "parse(%u) = %u, long byte count indicates success and skip "
                 "of the next %u bytes\n",
-                (unsigned)len, (unsigned)parsed, (unsigned)(parsed - len));
+                (unsigned)bytes, (unsigned)parsed, (unsigned)(parsed - bytes));
       } else {
         fprintf(stderr,
                 "parse(%u) = %u, short byte count indicates failure; "
                 "last %u bytes were not consumed\n",
-                (unsigned)len, (unsigned)parsed, (unsigned)(len - parsed));
+                (unsigned)bytes, (unsigned)parsed, (unsigned)(bytes - parsed));
       }
     }
 
-    if (status->ok() != (parsed >= len)) {
-      if (status->ok()) {
+    if (status_.ok() != (parsed >= bytes)) {
+      if (status_.ok()) {
         fprintf(stderr,
                 "Error: decode function returned short byte count but set no "
                 "error status\n");
@@ -69,17 +100,55 @@ bool parse_buffer(upb::BytesSink* sink, void* subc, const char* buf,
                 "Error: decode function returned complete byte count but set "
                 "error status\n");
       }
-      fprintf(stderr, "Status: %s, parsed=%u, len=%u\n",
-              status->error_message(), (unsigned)parsed, (unsigned)len);
+      fprintf(stderr, "Status: %s, parsed=%u, bytes=%u\n",
+              status_.error_message(), (unsigned)parsed, (unsigned)bytes);
       ASSERT(false);
     }
 
-    if (!status->ok())
+    if (!status_.ok())
       return false;
 
-    *ofs += parsed;
+    if (parsed > bytes && skip_until_ >= 0) {
+      skip_until_ = ofs_ + parsed;
+    }
+
+    ofs_ += UPB_MIN(parsed, bytes);
+
+    return true;
+  }
+
+  void ResetBytesSink(upb::BytesSink* sink) {
+    sink_ = sink;
   }
-  return true;
-}
+
+  const upb::Status& status() { return status_; }
+
+  size_t ofs() { return ofs_; }
+  upb::Environment* env() { return &env_; }
+
+  bool SkippedWithNull() { return skip_until_ > 0; }
+
+ private:
+  upb::Environment env_;
+  upb::Status status_;
+  upb::BytesSink* sink_;
+  const char* buf_;
+  size_t len_;
+  bool verbose_;
+  size_t ofs_;
+  void *subc_;
+
+  // When our parse call returns a value greater than the number of bytes
+  // we passed in, the decoder is indicating to us that the next N bytes
+  // in the stream are not needed and can be skipped.  The user is allowed
+  // to pass a NULL buffer for those N bytes.
+  //
+  // skip_until_ is initially set to 0 if we should do this NULL-buffer
+  // skipping or -1 if we should not.  If we are open to doing NULL-buffer
+  // skipping and we get an opportunity to do it, we set skip_until to the
+  // stream offset where we can skip until.  The user can then test whether
+  // this happened by testing SkippedWithNull().
+  int skip_until_;
+};
 
 #endif