More work on decoder and stdio bytesrc/bytesink.

Makefile

@@ -63,10 +63,10 @@ SRC=core/upb.c \
   descriptor/descriptor.c \
   core/upb_def.c \
   stream/upb_decoder.c \
+  stream/upb_stdio.c \
+  stream/upb_textprinter.c
 #  core/upb_msg.c \
-#  stream/upb_stdio.c \
 #  stream/upb_strstream.c \
-#  stream/upb_textprinter.c
 
 $(SRC): perf-cppflags
 # Parts of core that are yet to be converted.
@@ -114,7 +114,7 @@ TESTS=tests/test_string \
     tests/test_table \
     tests/test_def \
     tests/test_stream \
-#    tests/test_decoder \
+    tests/test_decoder \
 #    tests/t.test_vs_proto2.googlemessage1 \
 #    tests/t.test_vs_proto2.googlemessage2 \
 #    tests/test.proto.pb

core/upb.c

@@ -41,16 +41,13 @@ const upb_type_info upb_types[] = {
 };
 
 void upb_seterr(upb_status *status, enum upb_status_code code,
-                const char *msg, ...)
-{
-  if(upb_ok(status)) {  // The first error is the most interesting.
-    status->code = code;
-    upb_string_recycle(&status->str);
-    va_list args;
-    va_start(args, msg);
-    upb_string_vprintf(status->str, msg, args);
-    va_end(args);
-  }
+                const char *msg, ...) {
+  status->code = code;
+  upb_string_recycle(&status->str);
+  va_list args;
+  va_start(args, msg);
+  upb_string_vprintf(status->str, msg, args);
+  va_end(args);
 }
 
 void upb_copyerr(upb_status *to, upb_status *from)

core/upb_stream.h

@@ -178,12 +178,16 @@ INLINE void upb_src_run(upb_src *src, upb_status *status);
 INLINE upb_strlen_t upb_bytesrc_read(upb_bytesrc *src, void *buf,
                                      upb_strlen_t count, upb_status *status);
 
-// Like upb_bytesrc_read(), but modifies "str" in-place, possibly aliasing
-// existing string data (which avoids a copy).  On the other hand, if
-// the data was *not* already in an existing string, this copies it into
-// a upb_string, and if the data needs to be put in a specific range of
-// memory (because eg. you need to put it into a different kind of string
-// object) then upb_bytesrc_get() could be better.
+// Like upb_bytesrc_read(), but modifies "str" in-place.  "str" MUST be newly
+// created or just recycled.  Returns "false" if no data was returned, either
+// due to error or EOF (check status for details).
+//
+// In comparison to upb_bytesrc_read(), this call can possibly alias existing
+// string data (which avoids a copy).  On the other hand, if the data was *not*
+// already in an existing string, this copies it into a upb_string, and if the
+// data needs to be put in a specific range of memory (because eg. you need to
+// put it into a different kind of string object) then upb_bytesrc_get() could
+// be better.
 INLINE bool upb_bytesrc_getstr(upb_bytesrc *src, upb_string *str,
                                upb_status *status);
 

stream/upb_decoder.c

@@ -126,6 +126,7 @@ static bool upb_getbuf(upb_decoder *d, void *data, size_t bytes_wanted,
     } else {
       // End-of-buffer.
       if (d->buf) d->buf_stream_offset += upb_string_len(d->buf);
+      upb_string_recycle(&d->buf);
       if (!upb_bytesrc_getstr(d->bytesrc, d->buf, d->status)) return false;
       s->ptr = upb_string_getrobuf(d->buf);
     }
@@ -295,7 +296,15 @@ void upb_decoder_run(upb_src *src, upb_status *status) {
   while(1) {
     // Parse/handle tag.
     upb_tag tag;
-    CHECK(upb_decode_tag(d, &state, &tag));
+    if (!upb_decode_tag(d, &state, &tag)) {
+      if (status->code == UPB_EOF && d->top == d->stack) {
+        // Normal end-of-file.
+        CHECK_FLOW(upb_dispatch_endmsg(&d->dispatcher));
+        return;
+      } else {
+        goto err;
+      }
+    }
 
     // Decode wire data.  Hopefully this branch will predict pretty well
     // since most types will read a varint here.
@@ -361,9 +370,6 @@ void upb_decoder_run(upb_src *src, upb_status *status) {
     CHECK_FLOW(upb_dispatch_value(&d->dispatcher, f, val));
   }
 
-  CHECK_FLOW(upb_dispatch_endmsg(&d->dispatcher));
-  return;
-
 err:
   if (upb_ok(status)) {
     upb_seterr(status, UPB_ERROR, "Callback returned UPB_BREAK");

stream/upb_stdio.c

@@ -23,44 +23,42 @@ void upb_stdio_reset(upb_stdio *stdio, FILE* file) {
   stdio->file = file;
 }
 
-static bool upb_stdio_read(upb_stdio *stdio, upb_string *str,
-                           int offset, size_t bytes_to_read) {
-  char *buf = upb_string_getrwbuf(str, offset + bytes_to_read) + offset;
-  size_t read = fread(buf, 1, bytes_to_read, stdio->file);
-  if(read < bytes_to_read) {
+static upb_strlen_t upb_stdio_read(upb_bytesrc *src, void *buf,
+                                   upb_strlen_t count, upb_status *status) {
+  upb_stdio *stdio = (upb_stdio*)src;
+  assert(count > 0);
+  size_t read = fread(buf, 1, count, stdio->file);
+  if(read < (size_t)count) {
     // Error or EOF.
-    stdio->bytesrc.eof = feof(stdio->file);
-    if(ferror(stdio->file)) {
-      upb_seterr(&stdio->bytesrc.status, UPB_STATUS_ERROR,
-                 "Error reading from stdio stream.");
-      return false;
+    if(feof(stdio->file)) {
+      upb_seterr(status, UPB_EOF, "");
+      return read;
+    } else if(ferror(stdio->file)) {
+      upb_seterr(status, UPB_ERROR, "Error reading from stdio stream.");
+      return -1;
     }
-    // Resize to actual read size.
-    upb_string_getrwbuf(str, offset + read);
   }
-  return true;
+  return read;
 }
 
-bool upb_stdio_get(upb_bytesrc *src, upb_string *str, upb_strlen_t minlen) {
-  // We ignore "minlen" since the stdio interfaces always return a full read
-  // unless they are at EOF.
-  (void)minlen;
-  return upb_stdio_read((upb_stdio*)src, str, 0, BLOCK_SIZE);
-}
-
-bool upb_stdio_append(upb_bytesrc *src, upb_string *str, upb_strlen_t len) {
-  return upb_stdio_read((upb_stdio*)src, str, upb_string_len(str), len);
+static bool upb_stdio_getstr(upb_bytesrc *src, upb_string *str,
+                             upb_status *status) {
+  upb_strlen_t read = upb_stdio_read(
+      src, upb_string_getrwbuf(str, BLOCK_SIZE), BLOCK_SIZE, status);
+  if (read <= 0) return false;
+  upb_string_getrwbuf(str, read);
+  return true;
 }
 
 int32_t upb_stdio_put(upb_bytesink *sink, upb_string *str) {
   upb_stdio *stdio = (upb_stdio*)((char*)sink - offsetof(upb_stdio, bytesink));
   upb_strlen_t len = upb_string_len(str);
-  size_t written = fwrite(upb_string_getrobuf(str), 1, len, stdio->file);
+  upb_strlen_t written = fwrite(upb_string_getrobuf(str), 1, len, stdio->file);
   if(written < len) {
     // Error or EOF.
     stdio->bytesink.eof = feof(stdio->file);
     if(ferror(stdio->file)) {
-      upb_seterr(&stdio->bytesink.status, UPB_STATUS_ERROR,
+      upb_seterr(&stdio->bytesink.status, UPB_ERROR,
                  "Error writing to stdio stream.");
       return 0;
     }
@@ -68,19 +66,19 @@ int32_t upb_stdio_put(upb_bytesink *sink, upb_string *str) {
   return written;
 }
 
-static upb_bytesrc_vtable upb_stdio_bytesrc_vtbl = {
-  (upb_bytesrc_get_fptr)upb_stdio_get,
-  (upb_bytesrc_append_fptr)upb_stdio_append,
-};
+upb_stdio *upb_stdio_new() {
+  static upb_bytesrc_vtbl bytesrc_vtbl = {
+    upb_stdio_read,
+    upb_stdio_getstr,
+  };
 
-static upb_bytesink_vtable upb_stdio_bytesink_vtbl = {
-  upb_stdio_put
-};
+  //static upb_bytesink_vtbl bytesink_vtbl = {
+  //  upb_stdio_put
+  //};
 
-upb_stdio *upb_stdio_new() {
   upb_stdio *stdio = malloc(sizeof(*stdio));
-  upb_bytesrc_init(&stdio->bytesrc, &upb_stdio_bytesrc_vtbl);
-  upb_bytesink_init(&stdio->bytesink, &upb_stdio_bytesink_vtbl);
+  upb_bytesrc_init(&stdio->bytesrc, &bytesrc_vtbl);
+  //upb_bytesink_init(&stdio->bytesink, &bytesink_vtbl);
   return stdio;
 }
 

stream/upb_textprinter.c

@@ -12,7 +12,6 @@
 #include "upb_string.h"
 
 struct _upb_textprinter {
-  upb_sink sink;
   upb_bytesink *bytesink;
   upb_string *str;
   int indent_depth;

stream/upb_textprinter.h

@@ -20,8 +20,7 @@ upb_textprinter *upb_textprinter_new();
 void upb_textprinter_free(upb_textprinter *p);
 void upb_textprinter_reset(upb_textprinter *p, upb_bytesink *sink,
                            bool single_line);
-
-upb_sink *upb_textprinter_sink(upb_textprinter *p);
+void upb_textprinter_sethandlers(upb_textprinter *p, upb_handlers *h);
 
 #ifdef __cplusplus
 }  /* extern "C" */