Tons of work: we're close to passing test_vs_proto2 again.

8 files changed, 387 insertions, 134 deletions

diff --git a/core/upb.h b/core/upb.h
index 243c7bc..779f85a 100644
--- a/core/upb.h
+++ b/core/upb.h
@@ -204,97 +204,6 @@ INLINE upb_atomic_refcount_t *upb_value_getrefcount(upb_value val) {
   return val.val.refcount;
 }
 
-// A pointer to a .proto value.  The owner must have an out-of-band way of
-// knowing the type, so it knows which union member to use.
-typedef union {
-  double *_double;
-  float *_float;
-  int32_t *int32;
-  int64_t *int64;
-  uint8_t *uint8;
-  uint32_t *uint32;
-  uint64_t *uint64;
-  bool *_bool;
-  upb_string **str;
-  upb_msg **msg;
-  upb_array **arr;
-  void *_void;
-} upb_valueptr;
-
-INLINE upb_valueptr upb_value_addrof(upb_value *val) {
-  upb_valueptr ptr = {&val->val._double};
-  return ptr;
-}
-
-// Reads or writes a upb_value from an address represented by a upb_value_ptr.
-// We need to know the value type to perform this operation, because we need to
-// know how much memory to copy (and for big-endian machines, we need to know
-// where in the upb_value the data goes).
-//
-// For little endian-machines where we didn't mind overreading, we could make
-// upb_value_read simply use memcpy().
-INLINE upb_value upb_value_read(upb_valueptr ptr, upb_fieldtype_t ft) {
-  upb_value val;
-
-#define CASE(t, member_name) \
-  case UPB_TYPE(t): val.val.member_name = *ptr.member_name; break;
-
-  switch(ft) {
-    CASE(DOUBLE,   _double)
-    CASE(FLOAT,    _float)
-    CASE(INT32,    int32)
-    CASE(INT64,    int64)
-    CASE(UINT32,   uint32)
-    CASE(UINT64,   uint64)
-    CASE(SINT32,   int32)
-    CASE(SINT64,   int64)
-    CASE(FIXED32,  uint32)
-    CASE(FIXED64,  uint64)
-    CASE(SFIXED32, int32)
-    CASE(SFIXED64, int64)
-    CASE(BOOL,     _bool)
-    CASE(ENUM,     int32)
-    CASE(STRING,   str)
-    CASE(BYTES,    str)
-    CASE(MESSAGE,  msg)
-    CASE(GROUP,    msg)
-    default: break;
-  }
-  return val;
-
-#undef CASE
-}
-
-INLINE void upb_value_write(upb_valueptr ptr, upb_value val,
-                            upb_fieldtype_t ft) {
-#define CASE(t, member_name) \
-  case UPB_TYPE(t): *ptr.member_name = val.val.member_name; break;
-
-  switch(ft) {
-    CASE(DOUBLE,   _double)
-    CASE(FLOAT,    _float)
-    CASE(INT32,    int32)
-    CASE(INT64,    int64)
-    CASE(UINT32,   uint32)
-    CASE(UINT64,   uint64)
-    CASE(SINT32,   int32)
-    CASE(SINT64,   int64)
-    CASE(FIXED32,  uint32)
-    CASE(FIXED64,  uint64)
-    CASE(SFIXED32, int32)
-    CASE(SFIXED64, int64)
-    CASE(BOOL,     _bool)
-    CASE(ENUM,     int32)
-    CASE(STRING,   str)
-    CASE(BYTES,    str)
-    CASE(MESSAGE,  msg)
-    CASE(GROUP,    msg)
-    default: break;
-  }
-
-#undef CASE
-}
-
 // Status codes used as a return value.  Codes >0 are not fatal and can be
 // resumed.
 enum upb_status_code {
@@ -317,10 +226,12 @@ enum upb_status_code {
 
 // TODO: consider adding error space and code, to let ie. errno be stored
 // as a proper code, or application-specific error codes.
-typedef struct {
+struct _upb_status {
   char code;
   upb_string *str;
-} upb_status;
+};
+
+typedef struct _upb_status upb_status;
 
 #define UPB_STATUS_INIT {UPB_OK, NULL}
 #define UPB_ERRORMSG_MAXLEN 256
diff --git a/core/upb_def.c b/core/upb_def.c
index 2eda89f..993d9e3 100644
--- a/core/upb_def.c
+++ b/core/upb_def.c
@@ -1019,6 +1019,7 @@ bool upb_symtab_add_defs(upb_symtab *s, upb_def **defs, int num_defs,
   upb_symtab_ent *tmptab_e;
   for(tmptab_e = upb_strtable_begin(&tmptab); tmptab_e;
       tmptab_e = upb_strtable_next(&tmptab, &tmptab_e->e)) {
+    printf("Inserting def for " UPB_STRFMT "\n", UPB_STRARG(tmptab_e->def->fqname));
     upb_symtab_ent *symtab_e =
         upb_strtable_lookup(&s->symtab, tmptab_e->def->fqname);
     if(symtab_e) {
@@ -1132,6 +1133,7 @@ void upb_symtab_addfds(upb_symtab *s, upb_src *src, upb_status *status)
   if (upb_ok(status))
     upb_symtab_add_defs(s, b.defs.defs, b.defs.len, false, status);
   upb_defbuilder_uninit(&b);
+  upb_handlers_uninit(&handlers);
 }
 
 
diff --git a/core/upb_glue.c b/core/upb_glue.c
new file mode 100644
index 0000000..7540a3a
--- /dev/null
+++ b/core/upb_glue.c
@@ -0,0 +1,38 @@
+/*
+ * upb - a minimalist implementation of protocol buffers.
+ *
+ * Copyright (c) 2010 Joshua Haberman.  See LICENSE for details.
+ */
+
+#include "upb_glue.h"
+#include "upb_msg.h"
+#include "upb_decoder.h"
+#include "upb_strstream.h"
+
+void upb_strtomsg(upb_string *str, upb_msg *msg, upb_msgdef *md,
+                  upb_status *status) {
+  upb_stringsrc strsrc;
+  upb_stringsrc_init(&strsrc);
+  upb_stringsrc_reset(&strsrc, str);
+
+  upb_decoder d;
+  upb_decoder_init(&d, md);
+  upb_decoder_reset(&d, upb_stringsrc_bytesrc(&strsrc));
+  upb_src *src = upb_decoder_src(&d);
+
+  upb_msgpopulator p;
+  upb_msgpopulator_init(&p);
+  upb_msgpopulator_reset(&p, msg, md);
+
+  upb_handlers h;
+  upb_handlers_init(&h);
+  upb_msgpopulator_register_handlers(&p, &h);
+  upb_src_sethandlers(src, &h);
+
+  upb_src_run(src, status);
+
+  upb_stringsrc_uninit(&strsrc);
+  upb_decoder_uninit(&d);
+  upb_msgpopulator_uninit(&p);
+  upb_handlers_uninit(&h);
+}
diff --git a/core/upb_glue.h b/core/upb_glue.h
new file mode 100644
index 0000000..61111f5
--- /dev/null
+++ b/core/upb_glue.h
@@ -0,0 +1,42 @@
+/*
+ * upb - a minimalist implementation of protocol buffers.
+ *
+ * upb's core components like upb_decoder and upb_msg are carefully designed to
+ * avoid depending on each other for maximum orthogonality.  In other words,
+ * you can use a upb_decoder to decode into *any* kind of structure; upb_msg is
+ * just one such structure.  You can use upb_decoder without having to link in
+ * upb_msg.
+ *
+ * However, for convenience we provide functions here for doing common
+ * operations like deserializing protobuf binary format into a upb_msg.  The
+ * compromise is that this file drags in almost all of upb as a dependency,
+ * which could be undesirable if you're trying to use a trimmed-down build of
+ * upb.
+ *
+ * Copyright (c) 2011 Joshua Haberman.  See LICENSE for details.
+ */
+
+#ifndef UPB_GLUE_H
+#define UPB_GLUE_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// Forward-declares so we don't have to include everything in this .h file.
+// Clients should use the regular, typedef'd names (eg. upb_string).
+struct _upb_string;
+struct _upb_msg;
+struct _upb_msgdef;
+struct _upb_status;
+
+// Decodes the given string, which must be in protobuf binary format, to the
+// given upb_msg with msgdef "md", storing the status of the operation in "s".
+void upb_strtomsg(struct _upb_string *str, struct _upb_msg *msg,
+                  struct _upb_msgdef *md, struct _upb_status *s);
+
+#ifdef __cplusplus
+}  /* extern "C" */
+#endif
+
+#endif
diff --git a/core/upb_msg.c b/core/upb_msg.c
index e9f863d..1ad73bc 100644
--- a/core/upb_msg.c
+++ b/core/upb_msg.c
@@ -10,6 +10,18 @@
 #include "upb_decoder.h"
 #include "upb_strstream.h"
 
+static uint32_t upb_round_up_pow2(uint32_t v) {
+  // http://graphics.stanford.edu/~seander/bithacks.html#RoundUpPowerOf2
+  v--;
+  v |= v >> 1;
+  v |= v >> 2;
+  v |= v >> 4;
+  v |= v >> 8;
+  v |= v >> 16;
+  v++;
+  return v;
+}
+
 static void upb_elem_free(upb_value v, upb_fielddef *f) {
   switch(f->type) {
     case UPB_TYPE(MESSAGE):
@@ -47,6 +59,58 @@ static void upb_field_unref(upb_value v, upb_fielddef *f) {
     upb_field_free(v, f);
 }
 
+
+/* upb_array ******************************************************************/
+
+upb_array *upb_array_new(void) {
+  upb_array *arr = malloc(sizeof(*arr));
+  upb_atomic_refcount_init(&arr->refcount, 1);
+  arr->size = 0;
+  arr->len = 0;
+  arr->ptr = NULL;
+  return arr;
+}
+
+void upb_array_recycle(upb_array **_arr, upb_fielddef *f) {
+  upb_array *arr = *_arr;
+  if(arr && upb_atomic_read(&arr->refcount) == 1) {
+    arr->len = 0;
+  } else {
+    upb_array_unref(arr, f);
+    *_arr = upb_array_new();
+  }
+}
+
+void _upb_array_free(upb_array *arr, upb_fielddef *f) {
+  if (upb_elem_ismm(f)) {
+    // Need to release refs on sub-objects.
+    upb_valuetype_t type = upb_elem_valuetype(f);
+    for (upb_arraylen_t i = 0; i < arr->size; i++) {
+      upb_valueptr p = _upb_array_getptr(arr, f, i);
+      upb_elem_unref(upb_value_read(p, type), f);
+    }
+  }
+  free(arr->ptr);
+  free(arr);
+}
+
+void upb_array_resize(upb_array *arr, upb_fielddef *f, upb_arraylen_t len) {
+  size_t type_size = upb_types[f->type].size;
+  upb_arraylen_t old_size = arr->size;
+  if (old_size < len) {
+    // Need to resize.
+    size_t new_size = upb_round_up_pow2(len);
+    arr->ptr = realloc(arr->ptr, new_size * type_size);
+    arr->size = new_size;
+    memset(arr->ptr + (old_size * type_size), 0,
+           (new_size - old_size) * type_size);
+  }
+  arr->len = len;
+}
+
+
+/* upb_msg ********************************************************************/
+
 upb_msg *upb_msg_new(upb_msgdef *md) {
   upb_msg *msg = malloc(md->size);
   // Clear all set bits and cached pointers.
@@ -67,35 +131,123 @@ void _upb_msg_free(upb_msg *msg, upb_msgdef *md) {
   free(msg);
 }
 
+void upb_msg_recycle(upb_msg **_msg, upb_msgdef *msgdef) {
+  upb_msg *msg = *_msg;
+  if(msg && upb_atomic_read(&msg->refcount) == 1) {
+    upb_msg_clear(msg, msgdef);
+  } else {
+    upb_msg_unref(msg, msgdef);
+    *_msg = upb_msg_new(msgdef);
+  }
+}
+
 INLINE void upb_msg_sethas(upb_msg *msg, upb_fielddef *f) {
   msg->data[f->field_index/8] |= (1 << (f->field_index % 8));
 }
 
+static upb_valueptr upb_msg_getappendptr(upb_msg *msg, upb_fielddef *f) {
+  upb_valueptr p = _upb_msg_getptr(msg, f);
+  if (upb_isarray(f)) {
+    // Create/recycle/resize the array if necessary, and find a pointer to
+    // a newly-appended element.
+    if (!upb_msg_has(msg, f)) {
+      upb_array_recycle(p.arr, f);
+      upb_msg_sethas(msg, f);
+    }
+    assert(*p.arr != NULL);
+    upb_arraylen_t oldlen = upb_array_len(*p.arr);
+    upb_array_resize(*p.arr, f, oldlen + 1);
+    p = _upb_array_getptr(*p.arr, f, oldlen);
+  }
+  return p;
+}
 
-upb_array *upb_array_new(void) {
-  upb_array *arr = malloc(sizeof(*arr));
-  upb_atomic_refcount_init(&arr->refcount, 1);
-  arr->size = 0;
-  arr->len = 0;
-  arr->elements._void = NULL;
-  return arr;
+static void upb_msg_appendval(upb_msg *msg, upb_fielddef *f, upb_value val) {
+  upb_valueptr p = upb_msg_getappendptr(msg, f);
+  if (upb_isstring(f)) {
+    // We do:
+    //  - upb_string_recycle(), upb_string_substr() instead of
+    //  - upb_string_unref(), upb_string_getref()
+    // because we have a convenient place of caching these string objects for
+    // reuse, where as the upb_src who is sending us these strings may not.
+    // This saves the upb_src from allocating new upb_strings all the time to
+    // give us.
+    //
+    // If you were using this to copy one upb_msg to another this would
+    // allocate string objects whereas a upb_string_getref could have avoided
+    // those allocations completely; if this is an issue, we could make it an
+    // option of the upb_msgpopulator which behavior is desired.
+    upb_string *src = upb_value_getstr(val);
+    upb_string_recycle(p.str);
+    upb_string_substr(*p.str, src, 0, upb_string_len(src));
+  } else {
+    upb_value_write(p, val, f->type);
+  }
 }
 
-void _upb_array_free(upb_array *arr, upb_fielddef *f) {
-  if (upb_elem_ismm(f)) {
-    // Need to release refs on sub-objects.
-    upb_valuetype_t type = upb_elem_valuetype(f);
-    for (upb_arraylen_t i = 0; i < arr->size; i++) {
-      upb_valueptr p = _upb_array_getptr(arr, f, i);
-      upb_elem_unref(upb_value_read(p, type), f);
-    }
+upb_msg *upb_msg_appendmsg(upb_msg *msg, upb_fielddef *f, upb_msgdef *msgdef) {
+  upb_valueptr p = upb_msg_getappendptr(msg, f);
+  if (upb_isarray(f) || !upb_msg_has(msg, f)) {
+    upb_msg_recycle(p.msg, msgdef);
+    upb_msg_clear(*p.msg, msgdef);
+    upb_msg_sethas(msg, f);
   }
-  if (arr->elements._void) free(arr->elements._void);
-  free(arr);
+  return *p.msg;
 }
 
-void upb_msg_register_handlers(upb_msg *msg, upb_msgdef *md,
-                               upb_handlers *handlers, bool merge) {
-  static upb_handlerset handlerset = {
+
+/* upb_msgpopulator ***********************************************************/
+
+void upb_msgpopulator_init(upb_msgpopulator *p) {
+  upb_status_init(&p->status);
+}
+
+void upb_msgpopulator_reset(upb_msgpopulator *p, upb_msg *m, upb_msgdef *md) {
+  p->top = p->stack;
+  p->limit = p->stack + sizeof(p->stack);
+  p->top->msg = m;
+  p->top->msgdef = md;
+}
+
+void upb_msgpopulator_uninit(upb_msgpopulator *p) {
+  upb_status_uninit(&p->status);
+}
+
+static upb_flow_t upb_msgpopulator_value(void *_p, upb_fielddef *f, upb_value val) {
+  upb_msgpopulator *p = _p;
+  upb_msg_appendval(p->top->msg, f, val);
+  return UPB_CONTINUE;
+}
+
+static upb_flow_t upb_msgpopulator_startsubmsg(void *_p, upb_fielddef *f,
+                                               upb_handlers *delegate_to) {
+  upb_msgpopulator *p = _p;
+  (void)delegate_to;
+  upb_msg *oldmsg = p->top->msg;
+  if (++p->top == p->limit) {
+    upb_seterr(&p->status, UPB_ERROR, "Exceeded maximum nesting");
+    return UPB_BREAK;
   }
+  upb_msgdef *msgdef = upb_downcast_msgdef(f->def);
+  p->top->msgdef = msgdef;
+  p->top->msg = upb_msg_appendmsg(oldmsg, f, msgdef);
+  return UPB_CONTINUE;
+}
+
+static upb_flow_t upb_msgpopulator_endsubmsg(void *_p) {
+  upb_msgpopulator *p = _p;
+  --p->top;
+  return UPB_CONTINUE;
+}
+
+void upb_msgpopulator_register_handlers(upb_msgpopulator *p, upb_handlers *h) {
+  static upb_handlerset handlerset = {
+    NULL,   // startmsg
+    NULL,   // endmsg
+    &upb_msgpopulator_value,
+    &upb_msgpopulator_startsubmsg,
+    &upb_msgpopulator_endsubmsg,
+  };
+  upb_register_handlerset(h, &handlerset);
+  upb_set_handler_closure(h, p, &p->status);
 }
diff --git a/core/upb_msg.h b/core/upb_msg.h
index 0569039..8e04c95 100644
--- a/core/upb_msg.h
+++ b/core/upb_msg.h
@@ -23,21 +23,119 @@
 extern "C" {
 #endif
 
+// A pointer to a .proto value.  The owner must have an out-of-band way of
+// knowing the type, so it knows which union member to use.
+typedef union {
+  double *_double;
+  float *_float;
+  int32_t *int32;
+  int64_t *int64;
+  uint8_t *uint8;
+  uint32_t *uint32;
+  uint64_t *uint64;
+  bool *_bool;
+  upb_string **str;
+  upb_msg **msg;
+  upb_array **arr;
+  void *_void;
+} upb_valueptr;
+
+INLINE upb_valueptr upb_value_addrof(upb_value *val) {
+  upb_valueptr ptr = {&val->val._double};
+  return ptr;
+}
+
+// Reads or writes a upb_value from an address represented by a upb_value_ptr.
+// We need to know the value type to perform this operation, because we need to
+// know how much memory to copy (and for big-endian machines, we need to know
+// where in the upb_value the data goes).
+//
+// For little endian-machines where we didn't mind overreading, we could make
+// upb_value_read simply use memcpy().
+INLINE upb_value upb_value_read(upb_valueptr ptr, upb_fieldtype_t ft) {
+  upb_value val;
+
+#ifdef NDEBUG
+#define CASE(t, member_name) \
+  case UPB_TYPE(t): val.val.member_name = *ptr.member_name; break;
+#else
+#define CASE(t, member_name) \
+  case UPB_TYPE(t): val.val.member_name = *ptr.member_name; val.type = ft; break;
+#endif
+
+  switch(ft) {
+    CASE(DOUBLE,   _double)
+    CASE(FLOAT,    _float)
+    CASE(INT32,    int32)
+    CASE(INT64,    int64)
+    CASE(UINT32,   uint32)
+    CASE(UINT64,   uint64)
+    CASE(SINT32,   int32)
+    CASE(SINT64,   int64)
+    CASE(FIXED32,  uint32)
+    CASE(FIXED64,  uint64)
+    CASE(SFIXED32, int32)
+    CASE(SFIXED64, int64)
+    CASE(BOOL,     _bool)
+    CASE(ENUM,     int32)
+    CASE(STRING,   str)
+    CASE(BYTES,    str)
+    CASE(MESSAGE,  msg)
+    CASE(GROUP,    msg)
+    default: assert(false);
+  }
+  return val;
+
+#undef CASE
+}
+
+INLINE void upb_value_write(upb_valueptr ptr, upb_value val,
+                            upb_fieldtype_t ft) {
+  assert(val.type == ft);
+#define CASE(t, member_name) \
+  case UPB_TYPE(t): *ptr.member_name = val.val.member_name; break;
+
+  switch(ft) {
+    CASE(DOUBLE,   _double)
+    CASE(FLOAT,    _float)
+    CASE(INT32,    int32)
+    CASE(INT64,    int64)
+    CASE(UINT32,   uint32)
+    CASE(UINT64,   uint64)
+    CASE(SINT32,   int32)
+    CASE(SINT64,   int64)
+    CASE(FIXED32,  uint32)
+    CASE(FIXED64,  uint64)
+    CASE(SFIXED32, int32)
+    CASE(SFIXED64, int64)
+    CASE(BOOL,     _bool)
+    CASE(ENUM,     int32)
+    CASE(STRING,   str)
+    CASE(BYTES,    str)
+    CASE(MESSAGE,  msg)
+    CASE(GROUP,    msg)
+    default: assert(false);
+  }
+
+#undef CASE
+}
+
 /* upb_array ******************************************************************/
 
 typedef uint32_t upb_arraylen_t;
 struct _upb_array {
   upb_atomic_refcount_t refcount;
+  // "len" and "size" are measured in elements, not bytes.
   upb_arraylen_t len;
   upb_arraylen_t size;
-  upb_valueptr elements;
+  char *ptr;
 };
 
 void _upb_array_free(upb_array *a, upb_fielddef *f);
 INLINE upb_valueptr _upb_array_getptr(upb_array *a, upb_fielddef *f,
                                       uint32_t elem) {
   upb_valueptr p;
-  p._void = &a->elements.uint8[elem * upb_types[f->type].size];
+  p._void = &a->ptr[elem * upb_types[f->type].size];
   return p;
 }
 
@@ -47,10 +145,18 @@ INLINE void upb_array_unref(upb_array *a, upb_fielddef *f) {
   if (upb_atomic_unref(&a->refcount)) _upb_array_free(a, f);
 }
 
+void upb_array_recycle(upb_array **arr, upb_fielddef *f);
+
 INLINE uint32_t upb_array_len(upb_array *a) {
   return a->len;
 }
 
+INLINE upb_value upb_array_get(upb_array *arr, upb_fielddef *f,
+                               upb_arraylen_t i) {
+  assert(i < upb_array_len(arr));
+  return upb_value_read(_upb_array_getptr(arr, f, i), f->type);
+}
+
 /* upb_msg ********************************************************************/
 
 struct _upb_msg {
@@ -74,20 +180,37 @@ INLINE void upb_msg_unref(upb_msg *msg, upb_msgdef *md) {
   if (msg && upb_atomic_unref(&msg->refcount)) _upb_msg_free(msg, md);
 }
 
+void upb_msg_recycle(upb_msg **msg, upb_msgdef *msgdef);
+
 // Tests whether the given field is explicitly set, or whether it will return a
 // default.
 INLINE bool upb_msg_has(upb_msg *msg, upb_fielddef *f) {
   return (msg->data[f->field_index/8] & (1 << (f->field_index % 8))) != 0;
 }
 
+INLINE upb_value upb_msg_get(upb_msg *msg, upb_fielddef *f) {
+  return upb_value_read(_upb_msg_getptr(msg, f), f->type);
+}
+
 // Unsets all field values back to their defaults.
 INLINE void upb_msg_clear(upb_msg *msg, upb_msgdef *md) {
   memset(msg->data, 0, md->set_flags_bytes);
 }
 
-// Registers a set of handlers that will populate this msgdef.
-void upb_msg_register_handlers(upb_msg *msg, upb_msgdef *md,
-                               upb_handlers *handlers);
+typedef struct {
+  upb_msg *msg;
+  upb_msgdef *msgdef;
+} upb_msgpopulator_frame;
+
+typedef struct {
+  upb_msgpopulator_frame stack[UPB_MAX_NESTING], *top, *limit;
+  upb_status status;
+} upb_msgpopulator;
+
+void upb_msgpopulator_init(upb_msgpopulator *p);
+void upb_msgpopulator_uninit(upb_msgpopulator *p);
+void upb_msgpopulator_reset(upb_msgpopulator *p, upb_msg *m, upb_msgdef *md);
+void upb_msgpopulator_register_handlers(upb_msgpopulator *p, upb_handlers *h);
 
 #ifdef __cplusplus
 }  /* extern "C" */
diff --git a/core/upb_stream_vtbl.h b/core/upb_stream_vtbl.h
index a6990bc..da3eac8 100644
--- a/core/upb_stream_vtbl.h
+++ b/core/upb_stream_vtbl.h
@@ -62,14 +62,10 @@ typedef struct {
 
 struct _upb_bytesrc {
   upb_bytesrc_vtbl *vtbl;
-  upb_status status;
-  bool eof;
 };
 
 struct _upb_bytesink {
   upb_bytesink_vtbl *vtbl;
-  upb_status status;
-  bool eof;
 };
 
 struct _upb_src {
@@ -78,14 +74,10 @@ struct _upb_src {
 
 INLINE void upb_bytesrc_init(upb_bytesrc *s, upb_bytesrc_vtbl *vtbl) {
   s->vtbl = vtbl;
-  s->eof = false;
-  upb_status_init(&s->status);
 }
 
 INLINE void upb_bytesink_init(upb_bytesink *s, upb_bytesink_vtbl *vtbl) {
   s->vtbl = vtbl;
-  s->eof = false;
-  upb_status_init(&s->status);
 }
 
 INLINE void upb_src_init(upb_src *s, upb_src_vtbl *vtbl) {
@@ -132,9 +124,6 @@ INLINE bool upb_bytesrc_getfullstr(upb_bytesrc *src, upb_string *str,
   return true;
 }
 
-INLINE upb_status *upb_bytesrc_status(upb_bytesrc *src) { return &src->status; }
-INLINE bool upb_bytesrc_eof(upb_bytesrc *src) { return src->eof; }
-
 
 // upb_bytesink
 INLINE upb_strlen_t upb_bytesink_write(upb_bytesink *sink, void *buf,
@@ -146,10 +135,6 @@ INLINE upb_strlen_t upb_bytesink_putstr(upb_bytesink *sink, upb_string *str, upb
   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);
diff --git a/core/upb_string.c b/core/upb_string.c
index c599728..bc5b772 100644
--- a/core/upb_string.c
+++ b/core/upb_string.c
@@ -56,7 +56,7 @@ static void upb_string_release(upb_string *str) {
 }
 
 void _upb_string_free(upb_string *str) {
-  if(str->cached_mem) free(str->cached_mem);
+  free(str->cached_mem);
   upb_string_release(str);
   free(str);
 }