Fleshed out upb_stdio and upb_textprinter.

test_decoder now compiles and links!  But it doesn't
work yet.

Makefile

@@ -27,7 +27,7 @@ rwildcard=$(strip $(foreach d,$(wildcard $1*),$(call rwildcard,$d/,$2)$(filter $
 CC=gcc
 CXX=g++
 CFLAGS=-std=c99
-INCLUDE=-Idescriptor -Icore -Itests -I.
+INCLUDE=-Idescriptor -Icore -Itests -Istream -I.
 CPPFLAGS=-Wall -Wextra -g $(INCLUDE) $(strip $(shell test -f perf-cppflags && cat perf-cppflags))
 LDLIBS=-lpthread
 
@@ -47,7 +47,7 @@ clean:
 
 # The core library (core/libupb.a)
 SRC=core/upb.c stream/upb_decoder.c core/upb_table.c core/upb_def.c core/upb_string.c \
-    core/upb_stream.c \
+    core/upb_stream.c stream/upb_stdio.c stream/upb_textprinter.c \
     descriptor/descriptor.c
 $(SRC): perf-cppflags
 # Parts of core that are yet to be converted.
@@ -90,7 +90,8 @@ tests/test.proto.pb: tests/test.proto
 
 TESTS=tests/test_string \
     tests/test_table \
-    tests/test_def
+    tests/test_def \
+    tests/test_decoder
 tests: $(TESTS)
 
 OTHER_TESTS=tests/tests \

core/upb_stream.h

@@ -111,7 +111,10 @@ bool upb_sink_putdef(upb_sink *sink, struct _upb_fielddef *def);
 bool upb_sink_putval(upb_sink *sink, upb_value val);
 bool upb_sink_putstr(upb_sink *sink, upb_string *str);
 
-// Ends a submessage.
+// Starts/ends a submessage.  upb_sink_startmsg may seem redundant, but a
+// client could have a submessage already serialized, and therefore put it
+// as a string instead of its individual elements.
+bool upb_sink_startmsg(upb_sink *sink);
 bool upb_sink_endmsg(upb_sink *sink);
 
 // Returns the current error status for the stream.

core/upb_stream_vtbl.h

@@ -5,6 +5,21 @@
  * interfaces.  Only components that are implementing these interfaces need
  * to worry about this file.
  *
+ * This is tedious; this is the place in upb where I most wish I had a C++
+ * feature.  In C++ the compiler would generate this all for me.  If there's
+ * any consolation, it's that I have a bit of flexibility you don't have in
+ * C++: I could, with preprocessor magic alone "de-virtualize" this interface
+ * for a particular source file.  Say I had a C file that called a upb_src,
+ * but didn't want to pay the virtual function overhead.  I could define:
+ *
+ *   #define upb_src_getdef(src) upb_decoder_getdef((upb_decoder*)src)
+ *   #define upb_src_stargmsg(src) upb_decoder_startmsg(upb_decoder*)src)
+ *   // etc.
+ *
+ * The source file is compatible with the regular upb_src interface, but here
+ * we bind it to a particular upb_src (upb_decoder), which could lead to
+ * improved performance at a loss of flexibility for this one upb_src client.
+ *
  * Copyright (c) 2010 Joshua Haberman.  See LICENSE for details.
  */
 
@@ -39,12 +54,13 @@ typedef bool (*upb_src_endmsg_fptr)(upb_src *src);
 // upb_sink.
 typedef bool (*upb_sink_putdef_fptr)(upb_sink *sink, struct _upb_fielddef *def);
 typedef bool (*upb_sink_putval_fptr)(upb_sink *sink, upb_value val);
+typedef bool (*upb_sink_putstr_fptr)(upb_sink *sink, upb_string *str);
 typedef bool (*upb_sink_startmsg_fptr)(upb_sink *sink);
 typedef bool (*upb_sink_endmsg_fptr)(upb_sink *sink);
 
 // upb_bytesrc.
-typedef upb_string *(*upb_bytesrc_get_fptr)(upb_bytesrc *src);
-typedef void (*upb_bytesrc_recycle_fptr)(upb_bytesrc *src, upb_string *str);
+typedef bool (*upb_bytesrc_get_fptr)(
+    upb_bytesrc *src, upb_string *str, upb_strlen_t minlen);
 typedef bool (*upb_bytesrc_append_fptr)(
     upb_bytesrc *src, upb_string *str, upb_strlen_t len);
 
@@ -61,12 +77,23 @@ typedef struct {
   upb_src_endmsg_fptr   endmsg;
 } upb_src_vtable;
 
+typedef struct {
+  upb_sink_putdef_fptr   putdef;
+  upb_sink_putval_fptr   putval;
+  upb_sink_putstr_fptr   putstr;
+  upb_sink_startmsg_fptr startmsg;
+  upb_sink_endmsg_fptr   endmsg;
+} upb_sink_vtable;
+
 typedef struct {
   upb_bytesrc_get_fptr     get;
   upb_bytesrc_append_fptr  append;
-  upb_bytesrc_recycle_fptr recycle;
 } upb_bytesrc_vtable;
 
+typedef struct {
+  upb_bytesink_put_fptr  put;
+} upb_bytesink_vtable;
+
 // "Base Class" definitions; components that implement these interfaces should
 // contain one of these structures.
 
@@ -74,9 +101,12 @@ struct upb_src {
   upb_src_vtable *vtbl;
   upb_status status;
   bool eof;
-#ifndef NDEBUG
-  int state;  // For debug-mode checking of API usage.
-#endif
+};
+
+struct upb_sink {
+  upb_sink_vtable *vtbl;
+  upb_status status;
+  bool eof;
 };
 
 struct upb_bytesrc {
@@ -85,13 +115,34 @@ struct upb_bytesrc {
   bool eof;
 };
 
+struct upb_bytesink {
+  upb_bytesink_vtable *vtbl;
+  upb_status status;
+  bool eof;
+};
+
 INLINE void upb_src_init(upb_src *s, upb_src_vtable *vtbl) {
   s->vtbl = vtbl;
   s->eof = false;
   upb_status_init(&s->status);
-#ifndef DEBUG
-  // TODO: initialize debug-mode checking.
-#endif
+}
+
+INLINE void upb_sink_init(upb_sink *s, upb_sink_vtable *vtbl) {
+  s->vtbl = vtbl;
+  s->eof = false;
+  upb_status_init(&s->status);
+}
+
+INLINE void upb_bytesrc_init(upb_bytesrc *s, upb_bytesrc_vtable *vtbl) {
+  s->vtbl = vtbl;
+  s->eof = false;
+  upb_status_init(&s->status);
+}
+
+INLINE void upb_bytesink_init(upb_bytesink *s, upb_bytesink_vtable *vtbl) {
+  s->vtbl = vtbl;
+  s->eof = false;
+  upb_status_init(&s->status);
 }
 
 // Implementation of virtual function dispatch.
@@ -136,6 +187,47 @@ bool upb_src_getuint64(upb_src *src, uint64_t *val);
 bool upb_src_getfloat(upb_src *src, float *val);
 bool upb_src_getdouble(upb_src *src, double *val);
 
+// upb_bytesrc
+INLINE bool upb_bytesrc_get(
+    upb_bytesrc *bytesrc, upb_string *str, upb_strlen_t minlen) {
+  return bytesrc->vtbl->get(bytesrc, str, minlen);
+}
+
+INLINE bool upb_bytesrc_append(
+    upb_bytesrc *bytesrc, upb_string *str, upb_strlen_t len) {
+  return bytesrc->vtbl->append(bytesrc, str, len);
+}
+
+// upb_sink
+INLINE bool upb_sink_putdef(upb_sink *sink, struct _upb_fielddef *def) {
+  return sink->vtbl->putdef(sink, def);
+}
+INLINE bool upb_sink_putval(upb_sink *sink, upb_value val) {
+  return sink->vtbl->putval(sink, val);
+}
+INLINE bool upb_sink_putstr(upb_sink *sink, upb_string *str) {
+  return sink->vtbl->putstr(sink, str);
+}
+INLINE bool upb_sink_startmsg(upb_sink *sink) {
+  return sink->vtbl->startmsg(sink);
+}
+INLINE bool upb_sink_endmsg(upb_sink *sink) {
+  return sink->vtbl->endmsg(sink);
+}
+
+INLINE upb_status *upb_sink_status(upb_sink *sink) { return &sink->status; }
+
+// upb_bytesink
+INLINE int32_t upb_bytesink_put(upb_bytesink *sink, upb_string *str) {
+  return sink->vtbl->put(sink, str);
+}
+INLINE upb_status *upb_bytesink_status(upb_bytesink *sink) {
+  return &sink->status;
+}
+
+// upb_bytesink
+
+
 #ifdef __cplusplus
 }  /* extern "C" */
 #endif

stream/upb_decoder.c

@@ -574,3 +574,7 @@ void upb_decoder_reset(upb_decoder *d, upb_bytesrc *bytesrc)
   d->buf_stream_offset = 0;
   d->buf_offset = 0;
 }
+
+upb_src *upb_decoder_src(upb_decoder *d) {
+  return &d->src;
+}

stream/upb_decoder.h

@@ -44,7 +44,7 @@ void upb_decoder_reset(upb_decoder *d, upb_bytesrc *bytesrc);
 
 // Returns a upb_src pointer by which the decoder can be used.  The returned
 // upb_src is invalidated by upb_decoder_reset() or upb_decoder_free().
-upb_src *upb_decoder_getsrc(upb_decoder *d);
+upb_src *upb_decoder_src(upb_decoder *d);
 
 #ifdef __cplusplus
 }  /* extern "C" */

stream/upb_stdio.c

@@ -6,6 +6,10 @@
 
 #include "upb_stdio.h"
 
+#include <stddef.h>
+#include <stdlib.h>
+#include "upb_string.h"
+
 // We can make this configurable if necessary.
 #define BLOCK_SIZE 4096
 
@@ -13,11 +17,15 @@ struct upb_stdio {
   upb_bytesrc bytesrc;
   upb_bytesink bytesink;
   FILE *file;
+};
+
+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, int bytes_to_read) {
-  char *buf = upb_string_getrwbuf(offset + bytes_to_read) + offset;
+                           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) {
     // Error or EOF.
@@ -44,7 +52,7 @@ 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);
 }
 
-int32_t upb_bytesink_put(upb_bytesink *sink, upb_string *str) {
+int32_t upb_stdio_put(upb_bytesink *sink, upb_string *str) {
   upb_stdio *stdio = (upb_stdio*)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);
@@ -59,3 +67,26 @@ int32_t upb_bytesink_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,
+};
+
+static upb_bytesink_vtable upb_stdio_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);
+  return stdio;
+}
+
+void upb_stdio_free(upb_stdio *stdio) {
+  free(stdio);
+}
+
+upb_bytesrc* upb_stdio_bytesrc(upb_stdio *stdio) { return &stdio->bytesrc; }
+upb_bytesink* upb_stdio_bytesink(upb_stdio *stdio) { return &stdio->bytesink; }

stream/upb_stdio.h

@@ -21,7 +21,7 @@ struct upb_stdio;
 typedef struct upb_stdio upb_stdio;
 
 // Creation/deletion.
-upb_stdio_ *upb_stdio__new();
+upb_stdio *upb_stdio_new();
 void upb_stdio_free(upb_stdio *stdio);
 
 // Reset/initialize the object for use.  The src or sink will call

stream/upb_text.c

@@ -1,93 +0,0 @@
-/*
- * upb - a minimalist implementation of protocol buffers.
- *
- * Copyright (c) 2009 Joshua Haberman.  See LICENSE for details.
- */
-
-#include <inttypes.h>
-#include "descriptor.h"
-#include "upb_text.h"
-#include "upb_data.h"
-
-bool upb_textprinter_putval(upb_textprinter *p, upb_value val) {
-  upb_string *p->str = upb_string_tryrecycle(p->str);
-#define CASE(fmtstr, member) upb_string_printf(p->str, fmtstr, val.member); break;
-  switch(type) {
-    case UPB_TYPE(DOUBLE):
-      CASE("%0.f", _double);
-    case UPB_TYPE(FLOAT):
-      CASE("%0.f", _float)
-    case UPB_TYPE(INT64):
-    case UPB_TYPE(SFIXED64):
-    case UPB_TYPE(SINT64):
-      CASE("%" PRId64, int64)
-    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 UPB_TYPE(ENUM):
-      CASE("%" PRIu32, uint32);
-    case UPB_TYPE(BOOL):
-      CASE("%hhu", _bool);
-  }
-  return upb_bytesink_put(p->str);
-}
-
-bool upb_textprinter_putstr(upb_textprinter *p, upb_string *str) {
-  upb_bytesink_put(UPB_STRLIT("\""));
-  // TODO: escaping.
-  upb_bytesink_put(str);
-  upb_bytesink_put(UPB_STRLIT("\""));
-}
-
-static void print_indent(upb_text_printer *p, FILE *stream)
-{
-  if(!p->single_line)
-    for(int i = 0; i < p->indent_depth; i++)
-      upb_bytesink_put(UPB_STRLIT("  "));
-}
-
-void upb_text_printfield(upb_text_printer *p, upb_strptr name,
-                         upb_field_type_t valtype, upb_value val,
-                         FILE *stream)
-{
-  print_indent(p, stream);
-  fprintf(stream, UPB_STRFMT ":", UPB_STRARG(name));
-  upb_text_printval(valtype, val, stream);
-  if(p->single_line)
-    fputc(' ', stream);
-  else
-    fputc('\n', stream);
-}
-
-void upb_textprinter_startmsg(upb_textprinter *p)
-{
-  print_indent(p, stream);
-  fprintf(stream, UPB_STRFMT " {", UPB_STRARG(submsg_type));
-  if(!p->single_line) fputc('\n', stream);
-  p->indent_depth++;
-}
-
-void upb_text_pop(upb_text_printer *p, FILE *stream)
-{
-  p->indent_depth--;
-  print_indent(p, stream);
-  fprintf(stream, "}\n");
-}
-
-static void printval(upb_text_printer *printer, upb_value v, upb_fielddef *f,
-                     FILE *stream)
-{
-  if(upb_issubmsg(f)) {
-    upb_text_push(printer, f->name, stream);
-    printmsg(printer, v.msg, upb_downcast_msgdef(f->def), stream);
-    upb_text_pop(printer, stream);
-  } else {
-    upb_text_printfield(printer, f->name, f->type, v, stream);
-  }
-}

stream/upb_text.h

@@ -1,36 +0,0 @@
-/*
- * upb - a minimalist implementation of protocol buffers.
- *
- * Copyright (c) 2009 Joshua Haberman.  See LICENSE for details.
- */
-
-#ifndef UPB_TEXT_H_
-#define UPB_TEXT_H_
-
-#include "upb.h"
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-typedef struct {
-  int indent_depth;
-  bool single_line;
-} upb_text_printer;
-
-INLINE void upb_text_printer_init(upb_text_printer *p, bool single_line) {
-  p->indent_depth = 0;
-  p->single_line = single_line;
-}
-void upb_text_printval(upb_field_type_t type, upb_value p, FILE *file);
-void upb_text_printfield(upb_text_printer *p, upb_strptr name,
-                         upb_field_type_t valtype, upb_value val, FILE *stream);
-void upb_text_push(upb_text_printer *p, upb_strptr submsg_type,
-                   FILE *stream);
-void upb_text_pop(upb_text_printer *p, FILE *stream);
-
-#ifdef __cplusplus
-}  /* extern "C" */
-#endif
-
-#endif  /* UPB_TEXT_H_ */

stream/upb_textprinter.c

@@ -0,0 +1,131 @@
+/*
+ * upb - a minimalist implementation of protocol buffers.
+ *
+ * Copyright (c) 2009 Joshua Haberman.  See LICENSE for details.
+ */
+
+#include "upb_textprinter.h"
+
+#include <inttypes.h>
+#include <malloc.h>
+#include "upb_def.h"
+#include "upb_string.h"
+
+struct _upb_textprinter {
+  upb_sink sink;
+  upb_bytesink *bytesink;
+  upb_string *str;
+  int indent_depth;
+  bool single_line;
+  upb_fielddef *f;
+};
+
+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 bool upb_textprinter_putval(upb_textprinter *p, upb_value val) {
+  p->str = upb_string_tryrecycle(p->str);
+#define CASE(fmtstr, member) upb_string_printf(p->str, fmtstr, val.member); break;
+  switch(p->f->type) {
+    case UPB_TYPE(DOUBLE):
+      CASE("%0.f", _double);
+    case UPB_TYPE(FLOAT):
+      CASE("%0.f", _float)
+    case UPB_TYPE(INT64):
+    case UPB_TYPE(SFIXED64):
+    case UPB_TYPE(SINT64):
+      CASE("%" PRId64, int64)
+    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 UPB_TYPE(ENUM):
+      CASE("%" PRIu32, uint32);
+    case UPB_TYPE(BOOL):
+      CASE("%hhu", _bool);
+  }
+  upb_bytesink_put(p->bytesink, p->str);
+  upb_textprinter_endfield(p);
+  return upb_ok(upb_bytesink_status(p->bytesink));
+}
+
+static bool upb_textprinter_putstr(upb_textprinter *p, upb_string *str) {
+  upb_bytesink_put(p->bytesink, UPB_STRLIT("\""));
+  // TODO: escaping.
+  upb_bytesink_put(p->bytesink, str);
+  upb_bytesink_put(p->bytesink, UPB_STRLIT("\""));
+  upb_textprinter_endfield(p);
+  return upb_ok(upb_bytesink_status(p->bytesink));
+}
+
+static void 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("  "));
+}
+
+static bool upb_textprinter_putdef(upb_textprinter *p, upb_fielddef *f)
+{
+  upb_textprinter_indent(p);
+  upb_bytesink_put(p->bytesink, f->name);
+  upb_bytesink_put(p->bytesink, UPB_STRLIT(":"));
+  p->f = f;
+  return upb_ok(upb_bytesink_status(p->bytesink));
+}
+
+static bool upb_textprinter_startmsg(upb_textprinter *p)
+{
+  upb_textprinter_indent(p);
+  upb_bytesink_put(p->bytesink, p->f->def->fqname);
+  upb_bytesink_put(p->bytesink, UPB_STRLIT(" {"));
+  if(!p->single_line) upb_bytesink_put(p->bytesink, UPB_STRLIT('\n'));
+  p->indent_depth++;
+  return upb_ok(upb_bytesink_status(p->bytesink));
+}
+
+static bool upb_textprinter_endmsg(upb_textprinter *p)
+{
+  p->indent_depth--;
+  upb_textprinter_indent(p);
+  upb_bytesink_put(p->bytesink, UPB_STRLIT("}"));
+  upb_textprinter_endfield(p);
+  return upb_ok(upb_bytesink_status(p->bytesink));
+}
+
+upb_sink_vtable upb_textprinter_vtbl = {
+  (upb_sink_putdef_fptr)upb_textprinter_putdef,
+  (upb_sink_putval_fptr)upb_textprinter_putval,
+  (upb_sink_putstr_fptr)upb_textprinter_putstr,
+  (upb_sink_startmsg_fptr)upb_textprinter_startmsg,
+  (upb_sink_endmsg_fptr)upb_textprinter_endmsg,
+};
+
+upb_textprinter *upb_textprinter_new() {
+  upb_textprinter *p = malloc(sizeof(*p));
+  upb_sink_init(&p->sink, &upb_textprinter_vtbl);
+  return p;
+}
+
+void upb_textprinter_free(upb_textprinter *p) {
+  free(p);
+}
+
+void upb_textprinter_reset(upb_textprinter *p, upb_bytesink *sink,
+                           bool single_line) {
+  p->bytesink = sink;
+  p->single_line = single_line;
+  p->indent_depth = 0;
+}
+
+upb_sink *upb_textprinter_sink(upb_textprinter *p) { return &p->sink; }

stream/upb_textprinter.h

@@ -0,0 +1,30 @@
+/*
+ * upb - a minimalist implementation of protocol buffers.
+ *
+ * Copyright (c) 2009 Joshua Haberman.  See LICENSE for details.
+ */
+
+#ifndef UPB_TEXT_H_
+#define UPB_TEXT_H_
+
+#include "upb_stream.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct _upb_textprinter;
+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);
+
+upb_sink *upb_textprinter_sink(upb_textprinter *p);
+
+#ifdef __cplusplus
+}  /* extern "C" */
+#endif
+
+#endif  /* UPB_TEXT_H_ */