Textprinter is compiling again.

4 files changed, 103 insertions, 58 deletions

diff --git a/core/upb_stream.h b/core/upb_stream.h
index 09e4025..aa23549 100644
--- a/core/upb_stream.h
+++ b/core/upb_stream.h
@@ -245,16 +245,26 @@ INLINE bool upb_value_getfullstr(upb_value val, upb_string *str,
 struct _upb_bytesink;
 typedef struct _upb_bytesink upb_bytesink;
 
-INLINE bool upb_bytesink_printf(upb_bytesink *sink, const char *fmt, ...);
+// TODO: Figure out how buffering should be handled.  Should the caller buffer
+// data and only call these functions when a buffer is full?  Seems most
+// efficient, but then buffering has to be configured in the caller, which
+// could be anything, which makes it hard to have a standard interface for
+// controlling buffering.
+//
+// The downside of having the bytesink buffer is efficiency: the caller is
+// making more (virtual) function calls, and the caller can't arrange to have
+// a big contiguous buffer.  The bytesink can do this, but will have to copy
+// to make the data contiguous.
+
+// Returns the number of bytes written.
+INLINE upb_strlen_t upb_bytesink_printf(upb_bytesink *sink, upb_status *status,
+                                        const char *fmt, ...);
 
 // Puts the given string, returning true if the operation was successful, otherwise
 // check "status" for details.  Ownership of the string is *not* passed; if
 // the callee wants a reference he must call upb_string_getref() on it.
-INLINE bool upb_bytesink_putstr(upb_bytesink *sink, upb_string *str,
-                                upb_status *status);
-
-// Returns the current error status for the stream.
-INLINE upb_status *upb_bytesink_status(upb_bytesink *sink);
+INLINE upb_strlen_t upb_bytesink_putstr(upb_bytesink *sink, upb_string *str,
+                                        upb_status *status);
 
 #include "upb_stream_vtbl.h"
 
diff --git a/core/upb_stream_vtbl.h b/core/upb_stream_vtbl.h
index ef655fd..8e8971f 100644
--- a/core/upb_stream_vtbl.h
+++ b/core/upb_stream_vtbl.h
@@ -34,8 +34,10 @@ typedef bool (*upb_bytesrc_getstr_fptr)(
 // upb_bytesink.
 typedef upb_strlen_t (*upb_bytesink_write_fptr)(
     upb_bytesink *bytesink, void *buf, upb_strlen_t count);
-typedef upb_strlen_t (*upb_bytesink_putstr_fptr)(
-    upb_bytesink *bytesink, upb_string *str);
+typedef bool (*upb_bytesink_putstr_fptr)(
+    upb_bytesink *bytesink, upb_string *str, upb_status *status);
+typedef upb_strlen_t (*upb_bytesink_vprintf_fptr)(
+    upb_status *status, const char *fmt, va_list args);
 
 // Vtables for the above interfaces.
 typedef struct {
@@ -44,8 +46,9 @@ typedef struct {
 } upb_bytesrc_vtbl;
 
 typedef struct {
-  upb_bytesink_write_fptr  write;
-  upb_bytesink_putstr_fptr putstr;
+  upb_bytesink_write_fptr   write;
+  upb_bytesink_putstr_fptr  putstr;
+  upb_bytesink_vprintf_fptr vprintf;
 } upb_bytesink_vtbl;
 
 typedef struct {
@@ -140,13 +143,21 @@ INLINE upb_strlen_t upb_bytesink_write(upb_bytesink *sink, void *buf,
 }
 
 INLINE upb_strlen_t upb_bytesink_putstr(upb_bytesink *sink, upb_string *str, upb_status *status) {
-  return sink->vtbl->putstr(sink, str);
+  return sink->vtbl->putstr(sink, str, status);
 }
 
 INLINE upb_status *upb_bytesink_status(upb_bytesink *sink) {
   return &sink->status;
 }
 
+INLINE upb_strlen_t upb_bytesink_printf(upb_bytesink *sink, upb_status *status, const char *fmt, ...) {
+  va_list args;
+  va_start(args, fmt);
+  upb_strlen_t ret = sink->vtbl->vprintf(status, fmt, args);
+  va_end(args);
+  return ret;
+}
+
 // upb_handlers
 struct _upb_handlers {
   upb_handlerset *set;
diff --git a/stream/upb_textprinter.c b/stream/upb_textprinter.c
index 2209173..7025494 100644
--- a/stream/upb_textprinter.c
+++ b/stream/upb_textprinter.c
@@ -15,34 +15,51 @@ struct _upb_textprinter {
   upb_bytesink *bytesink;
   int indent_depth;
   bool single_line;
-  upb_fielddef *f;
+  upb_status status;
 };
 
-static void upb_textprinter_indent(upb_textprinter *p)
+#define CHECK(x) if ((x) < 0) goto err;
+
+static int upb_textprinter_indent(upb_textprinter *p)
 {
   if(!p->single_line)
     for(int i = 0; i < p->indent_depth; i++)
-      upb_bytesink_put(p->bytesink, UPB_STRLIT("  "));
+      CHECK(upb_bytesink_putstr(p->bytesink, UPB_STRLIT("  "), &p->status));
+  return 0;
+err:
+  return -1;
 }
 
-static void upb_textprinter_endfield(upb_textprinter *p) {
-  if(p->single_line)
-    upb_bytesink_put(p->bytesink, UPB_STRLIT(" "));
-  else
-    upb_bytesink_put(p->bytesink, UPB_STRLIT("\n"));
+static int upb_textprinter_startfield(upb_textprinter *p, upb_fielddef *f) {
+  upb_textprinter_indent(p);
+  CHECK(upb_bytesink_printf(p->bytesink, &p->status, UPB_STRFMT ": ", UPB_STRARG(f->name)));
+  return 0;
+err:
+  return -1;
+}
+
+static int upb_textprinter_endfield(upb_textprinter *p) {
+  if(p->single_line) {
+    CHECK(upb_bytesink_putstr(p->bytesink, UPB_STRLIT(" "), &p->status));
+  } else {
+    CHECK(upb_bytesink_putstr(p->bytesink, UPB_STRLIT("\n"), &p->status));
+  }
+  return 0;
+err:
+  return -1;
 }
 
 static upb_flow_t upb_textprinter_value(void *_p, upb_fielddef *f,
                                         upb_value val) {
   upb_textprinter *p = _p;
-  upb_textprinter_indent(p);
-  upb_bytesink_printf(p->bytesink, UPB_STRFMT ": ", UPB_STRARG(f->name));
-#define CASE(fmtstr, member) upb_bytesink_printf(p->bytesink, fmtstr, val.member); break;
-  switch(p->f->type) {
+  upb_textprinter_startfield(p, f);
+#define CASE(fmtstr, member) \
+    CHECK(upb_bytesink_printf(p->bytesink, &p->status, fmtstr, upb_value_get ## member(val))); break;
+  switch(f->type) {
     case UPB_TYPE(DOUBLE):
-      CASE("%0.f", _double);
+      CASE("%0.f", double);
     case UPB_TYPE(FLOAT):
-      CASE("%0.f", _float)
+      CASE("%0.f", float)
     case UPB_TYPE(INT64):
     case UPB_TYPE(SFIXED64):
     case UPB_TYPE(SINT64):
@@ -50,40 +67,48 @@ static upb_flow_t upb_textprinter_value(void *_p, upb_fielddef *f,
     case UPB_TYPE(UINT64):
     case UPB_TYPE(FIXED64):
       CASE("%" PRIu64, uint64)
-    case UPB_TYPE(INT32):
-    case UPB_TYPE(SFIXED32):
-    case UPB_TYPE(SINT32):
-      CASE("%" PRId32, int32)
     case UPB_TYPE(UINT32):
     case UPB_TYPE(FIXED32):
       CASE("%" PRIu32, uint32);
     case UPB_TYPE(ENUM): {
-      upb_enumdef *enum_def;
-      upb_string *enum_label;
-       (enum_def = upb_downcast_enumdef(p->f->def)) != NULL &&
-       (enum_label = upb_enumdef_iton(enum_def, val.int32)) != NULL) {
-      // This is an enum value for which we found a corresponding string.
-      upb_bytesink_put(p->bytesink, enum_label);
-      CASE("%" PRIu32, uint32);
+      upb_enumdef *enum_def = upb_downcast_enumdef(f->def);
+      upb_string *enum_label =
+          upb_enumdef_iton(enum_def, upb_value_getint32(val));
+      if (enum_label) {
+        // We found a corresponding string for this enum.  Otherwise we fall
+        // through to the int32 code path.
+        CHECK(upb_bytesink_putstr(p->bytesink, enum_label, &p->status));
+        break;
+      }
     }
+    case UPB_TYPE(INT32):
+    case UPB_TYPE(SFIXED32):
+    case UPB_TYPE(SINT32):
+      CASE("%" PRId32, int32)
     case UPB_TYPE(BOOL):
-      CASE("%hhu", _bool);
+      CASE("%hhu", bool);
     case UPB_TYPE(STRING):
     case UPB_TYPE(BYTES):
-      upb_bytesink_put(p->bytesink, UPB_STRLIT(": \""));
-      upb_bytesink_put(p->bytesink, str);
-      upb_bytesink_put(p->bytesink, UPB_STRLIT("\""));
+      // TODO: escaping.
+      CHECK(upb_bytesink_putstr(p->bytesink, UPB_STRLIT(": \""), &p->status));
+      CHECK(upb_bytesink_putstr(p->bytesink, upb_value_getstr(val), &p->status))
+      CHECK(upb_bytesink_putstr(p->bytesink, UPB_STRLIT("\""), &p->status));
       break;
   }
   upb_textprinter_endfield(p);
   return UPB_CONTINUE;
+err:
+  return UPB_BREAK;
 }
 
-static upb_flow_t upb_textprinter_startsubmsg(void *_p, upb_fielddef *f) {
+static upb_flow_t upb_textprinter_startsubmsg(void *_p, upb_fielddef *f,
+                                              upb_handlers *delegate_to) {
+  (void)delegate_to;
   upb_textprinter *p = _p;
+  upb_textprinter_startfield(p, f);
   p->indent_depth++;
-  upb_bytesink_put(p->bytesink, UPB_STRLIT(" {"));
-  if(!p->single_line) upb_bytesink_put(p->bytesink, UPB_STRLIT("\n"));
+  upb_bytesink_putstr(p->bytesink, UPB_STRLIT(" {"), &p->status);
+  if(!p->single_line) upb_bytesink_putstr(p->bytesink, UPB_STRLIT("\n"), &p->status);
   return UPB_CONTINUE;
 }
 
@@ -92,21 +117,13 @@ static upb_flow_t upb_textprinter_endsubmsg(void *_p)
   upb_textprinter *p = _p;
   p->indent_depth--;
   upb_textprinter_indent(p);
-  upb_bytesink_put(p->bytesink, UPB_STRLIT("}"));
+  upb_bytesink_putstr(p->bytesink, UPB_STRLIT("}"), &p->status);
   upb_textprinter_endfield(p);
   return UPB_CONTINUE;
 }
 
 upb_textprinter *upb_textprinter_new() {
-  static upb_handlerset handlers = {
-    NULL,  // startmsg
-    NULL,  // endmsg
-    upb_textprinter_putval,
-    upb_textprinter_startsubmsg,
-    upb_textprinter_endsubmsg,
-  };
   upb_textprinter *p = malloc(sizeof(*p));
-  upb_byte_init(&p->sink, &upb_textprinter_vtbl);
   return p;
 }
 
@@ -114,11 +131,18 @@ void upb_textprinter_free(upb_textprinter *p) {
   free(p);
 }
 
-void upb_textprinter_reset(upb_textprinter *p, upb_bytesink *sink,
-                           bool single_line) {
+void upb_textprinter_reset(upb_textprinter *p, upb_handlers *handlers,
+                           upb_bytesink *sink, bool single_line) {
+  static upb_handlerset handlerset = {
+    NULL,  // startmsg
+    NULL,  // endmsg
+    upb_textprinter_value,
+    upb_textprinter_startsubmsg,
+    upb_textprinter_endsubmsg,
+  };
   p->bytesink = sink;
   p->single_line = single_line;
   p->indent_depth = 0;
+  upb_register_handlerset(handlers, &handlerset);
+  upb_set_handler_closure(handlers, p, &p->status);
 }
-
-upb_sink *upb_textprinter_sink(upb_textprinter *p) { return &p->sink; }
diff --git a/stream/upb_textprinter.h b/stream/upb_textprinter.h
index b40d9fa..a880626 100644
--- a/stream/upb_textprinter.h
+++ b/stream/upb_textprinter.h
@@ -18,8 +18,8 @@ typedef struct _upb_textprinter upb_textprinter;
 
 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);
+void upb_textprinter_reset(upb_textprinter *p, upb_handlers *handlers,
+                           upb_bytesink *sink, bool single_line);
 void upb_textprinter_sethandlers(upb_textprinter *p, upb_handlers *h);
 
 #ifdef __cplusplus