Use a status object for errors so a message can be returned.

Also delay deletion of subfields until the entire message is
deleted.

Makefile

@@ -28,7 +28,7 @@ CC=gcc
 CXX=g++
 CFLAGS=-std=c99
 INCLUDE=-Idescriptor -Isrc -Itests -I.
-CPPFLAGS=-Wall -Wextra -Werror -g $(INCLUDE) $(strip $(shell test -f perf-cppflags && cat perf-cppflags))
+CPPFLAGS=-Wall -Wextra -g $(INCLUDE) $(strip $(shell test -f perf-cppflags && cat perf-cppflags))
 LDLIBS=-lpthread
 
 LIBUPB=src/libupb.a
@@ -47,7 +47,8 @@ clean:
 # The core library (src/libupb.a)
 SRC=src/upb.c src/upb_parse.c src/upb_table.c src/upb_msg.c src/upb_mm.c \
     src/upb_enum.c src/upb_context.c src/upb_string.c src/upb_text.c \
-    src/upb_serialize.c descriptor/descriptor.c
+    descriptor/descriptor.c
+    #src/upb_serialize.c descriptor/descriptor.c
 STATICOBJ=$(patsubst %.c,%.o,$(SRC))
 SHAREDOBJ=$(patsubst %.c,%.lo,$(SRC))
 # building shared objects is like building static ones, except -fPIC is added.

benchmarks/parsetostruct.upb_table.c

@@ -9,26 +9,30 @@ static struct upb_context *c;
 static struct upb_string *str;
 static struct upb_msgdef *def;
 static struct upb_msg *msgs[NUM_MESSAGES];
+static struct upb_msgparser *mp;
 
 static bool initialize()
 {
   // Initialize upb state, parse descriptor.
+  struct upb_status status = UPB_STATUS_INIT;
   c = upb_context_new();
   struct upb_string *fds = upb_strreadfile(MESSAGE_DESCRIPTOR_FILE);
   if(!fds) {
-    fprintf(stderr, "Couldn't read " MESSAGE_DESCRIPTOR_FILE ".\n");
+    fprintf(stderr, "Couldn't read " MESSAGE_DESCRIPTOR_FILE ": %s.\n",
+            status.msg);
     return false;
   }
-  if(!upb_context_parsefds(c, fds)) {
-    fprintf(stderr, "Error importing " MESSAGE_DESCRIPTOR_FILE ".\n");
+  upb_context_parsefds(c, fds, &status);
+  if(!upb_ok(&status)) {
+    fprintf(stderr, "Error importing " MESSAGE_DESCRIPTOR_FILE ": %s.\n",
+            status.msg);
     return false;
   }
   upb_string_unref(fds);
 
   struct upb_string *proto_name = upb_strdupc(MESSAGE_NAME);
   struct upb_symtab_entry e;
-  upb_status_t success = upb_context_lookup(c, proto_name, &e);
-  if(!success || e.type != UPB_SYM_MESSAGE) {
+  if(!upb_context_lookup(c, proto_name, &e) || e.type != UPB_SYM_MESSAGE) {
     fprintf(stderr, "Error finding symbol '" UPB_STRFMT "'.\n",
             UPB_STRARG(proto_name));
     return false;
@@ -45,6 +49,7 @@ static bool initialize()
     fprintf(stderr, "Error reading " MESSAGE_FILE "\n");
     return false;
   }
+  mp = upb_msgparser_new(def);
   return true;
 }
 
@@ -54,14 +59,18 @@ static void cleanup()
     upb_msg_unref(msgs[i]);
   upb_string_unref(str);
   upb_context_unref(c);
+  upb_msgparser_free(mp);
 }
 
 static size_t run(int i)
 {
-  upb_status_t status;
-  status = upb_msg_parsestr(msgs[i%NUM_MESSAGES], str->ptr, str->byte_len);
-  if(status != UPB_STATUS_OK) {
-    fprintf(stderr, "Error. :(  error=%d\n", status);
+  struct upb_status status = UPB_STATUS_INIT;
+  struct upb_msg *msg = msgs[i%NUM_MESSAGES];
+  upb_msgparser_reset(mp, msg, false);
+  upb_msg_clear(msg);
+  upb_msgparser_parse(mp, str->ptr, str->byte_len, &status);
+  if(!upb_ok(&status)) {
+    fprintf(stderr, "Parse error: %s\n", status.msg);
     return 0;
   }
   return str->byte_len;

lang_ext/python/setup.py

@@ -6,7 +6,8 @@ setup(name='upb',
           Extension('upb.cext', ['definition.c', 'pb.c', 'cext.c'],
               include_dirs=['../../src', '../../descriptor'],
               define_macros=[("UPB_USE_PTHREADS", 1),
-                             ("UPB_UNALIGNED_READS_OK", 1)],
+                             ("UPB_UNALIGNED_READS_OK", 1),
+                             ("UPB_THREAD_UNSAFE", 1)],
               library_dirs=['../../src'],
               libraries=['upb_pic'],
           ),

src/upb.c

@@ -5,6 +5,7 @@
  *
  */
 
+#include <stdarg.h>
 #include <stddef.h>
 
 #include "upb.h"
@@ -35,3 +36,14 @@ struct upb_type_info upb_type_info[] = {
   TYPE_INFO(BYTES,    UPB_WIRE_TYPE_DELIMITED,   struct upb_string*)
 };
 
+void upb_seterr(struct 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;
+    va_list args;
+    va_start(args, msg);
+    vsnprintf(status->msg, UPB_ERRORMSG_MAXLEN, msg, args);
+    va_end(args);
+  }
+}

src/upb.h

@@ -34,7 +34,7 @@ extern "C" {
 
 // Nested type names are separated by periods.
 #define UPB_SYMBOL_SEPARATOR '.'
-#define UPB_SYMBOL_MAX_LENGTH 256
+#define UPB_SYMBOL_MAXLEN 128
 
 #define UPB_INDEX(base, i, m) (void*)((char*)(base) + ((i)*(m)))
 
@@ -230,7 +230,7 @@ union upb_symbol_ref {
 
 // Status codes used as a return value.  Codes >0 are not fatal and can be
 // resumed.
-typedef enum upb_status {
+enum upb_status_code {
   UPB_STATUS_OK = 0,
 
   // The input byte stream ended in the middle of a record.
@@ -239,28 +239,32 @@ typedef enum upb_status {
   // The user value callback opted to stop parsing.
   UPB_STATUS_USER_CANCELLED = 2,
 
-  // A varint did not terminate before hitting 64 bits.
-  UPB_ERROR_UNTERMINATED_VARINT = -1,
+  // An unrecoverable error occurred.
+  UPB_STATUS_ERROR = -1,
 
-  // A submessage or packed array ended in the middle of data.
-  UPB_ERROR_BAD_SUBMESSAGE_END = -2,
+  // A varint went for 10 bytes without terminating.
+  UPB_ERROR_UNTERMINATED_VARINT = -2
+};
 
-  // Input was nested more than UPB_MAX_NESTING deep.
-  UPB_ERROR_STACK_OVERFLOW = -3,
+#define UPB_ERRORMSG_MAXLEN 256
+struct upb_status {
+  enum upb_status_code code;
+  char msg[UPB_ERRORMSG_MAXLEN];
+};
 
-  // The input data caused the pb's offset (a size_t) to overflow.
-  UPB_ERROR_OVERFLOW = -4,
+#define UPB_STATUS_INIT {UPB_STATUS_OK, ""}
 
-  // An "end group" tag was encountered in an inappropriate place.
-  UPB_ERROR_SPURIOUS_END_GROUP = -5,
+INLINE bool upb_ok(struct upb_status *status) {
+  return status->code == UPB_STATUS_OK;
+}
 
-  UPB_ERROR_ILLEGAL = -6
-} upb_status_t;
+INLINE void upb_reset(struct upb_status *status) {
+  status->code = UPB_STATUS_OK;
+  status->msg[0] = '\0';
+}
 
-#define UPB_CHECK(func) do { \
-  upb_status_t status = func; \
-  if(status != UPB_STATUS_OK) return status; \
-  } while (0)
+void upb_seterr(struct upb_status *status, enum upb_status_code code,
+                const char *msg, ...);
 
 #ifdef __cplusplus
 }  /* extern "C" */

src/upb_array.h

@@ -70,14 +70,13 @@ INLINE uint32_t upb_round_up_to_pow2(uint32_t v)
 INLINE union upb_value_ptr upb_array_append(struct upb_array *arr)
 {
   size_t size = upb_type_info[arr->fielddef->type].size;
-  upb_arraylen_t oldlen = arr->len;
-  if(oldlen == arr->size) {
-    arr->size = UPB_MAX(4, upb_round_up_to_pow2(oldlen+1));
+  if(arr->len == arr->size) {
+    arr->size = UPB_MAX(4, upb_round_up_to_pow2(arr->len + 1));
     arr->elements._void = realloc(arr->elements._void, arr->size * size);
-    memset((char*)arr->elements._void + (arr->len*size), 0, (arr->size - arr->len) * size);
+    memset((char*)arr->elements._void + (arr->len * size), 0,
+           (arr->size - arr->len) * size);
   }
-  arr->len++;
-  return upb_array_getelementptr(arr, oldlen);
+  return upb_array_getelementptr(arr, arr->len++);
 }
 
 INLINE void upb_array_truncate(struct upb_array *arr)

src/upb_context.c

@@ -20,9 +20,10 @@ static int my_memrchr(char *data, char c, size_t len)
   return off;
 }
 
-bool addfd(struct upb_strtable *addto, struct upb_strtable *existingdefs,
+void addfd(struct upb_strtable *addto, struct upb_strtable *existingdefs,
            google_protobuf_FileDescriptorProto *fd, bool sort,
-           struct upb_context *context);
+           struct upb_context *context,
+           struct upb_status *status);
 
 struct upb_context *upb_context_new()
 {
@@ -34,7 +35,10 @@ struct upb_context *upb_context_new()
   /* Add all the types in descriptor.proto so we can parse descriptors. */
   google_protobuf_FileDescriptorProto *fd =
       upb_file_descriptor_set->file->elements[0]; /* We know there is only 1. */
-  if(!addfd(&c->psymtab, &c->symtab, fd, false, c)) {
+  struct upb_status status = UPB_STATUS_INIT;
+  addfd(&c->psymtab, &c->symtab, fd, false, c, &status);
+  if(!upb_ok(&status)) {
+    fprintf(stderr, "Failed to initialize upb: %s.\n", status.msg);
     assert(false);
     return NULL;  /* Indicates that upb is buggy or corrupt. */
   }
@@ -109,7 +113,7 @@ static struct upb_symtab_entry *resolve(struct upb_strtable *t,
                                         struct upb_string *base,
                                         struct upb_string *symbol)
 {
-  if(base->byte_len + symbol->byte_len + 1 >= UPB_SYMBOL_MAX_LENGTH ||
+  if(base->byte_len + symbol->byte_len + 1 >= UPB_SYMBOL_MAXLEN ||
      symbol->byte_len == 0) return NULL;
 
   if(symbol->ptr[0] == UPB_SYMBOL_SEPARATOR) {
@@ -119,7 +123,7 @@ static struct upb_symtab_entry *resolve(struct upb_strtable *t,
     return upb_strtable_lookup(t, &sym_str);
   } else {
     /* Remove components from base until we find an entry or run out. */
-    char sym[UPB_SYMBOL_MAX_LENGTH+1];
+    char sym[UPB_SYMBOL_MAXLEN+1];
     struct upb_string sym_str = {.ptr = sym};
     int baselen = base->byte_len;
     while(1) {
@@ -180,20 +184,28 @@ static struct upb_string join(struct upb_string *base, struct upb_string *name)
   return joined;
 }
 
-static bool insert_enum(struct upb_strtable *t,
+static void insert_enum(struct upb_strtable *t,
                         google_protobuf_EnumDescriptorProto *ed,
                         struct upb_string *base,
-                        struct upb_context *c)
+                        struct upb_context *c,
+                        struct upb_status *status)
 {
-  if(!ed->set_flags.has.name) return false;
+  if(!ed->set_flags.has.name) {
+    upb_seterr(status, UPB_STATUS_ERROR,
+               "enum in context '" UPB_STRFMT "' does not have a name",
+               UPB_STRARG(base));
+    return;
+  }
 
   /* We own this and must free it on destruct. */
   struct upb_string fqname = join(base, ed->name);
 
-  /* Redefinition within a FileDescriptorProto is not allowed. */
   if(upb_strtable_lookup(t, &fqname)) {
+    upb_seterr(status, UPB_STATUS_ERROR,
+               "attempted to redefine symbol '" UPB_STRFMT "'",
+               UPB_STRARG(&fqname));
     free(fqname.ptr);
-    return false;
+    return;
   }
 
   struct upb_symtab_entry e;
@@ -202,74 +214,81 @@ static bool insert_enum(struct upb_strtable *t,
   e.ref._enum = malloc(sizeof(*e.ref._enum));
   upb_enum_init(e.ref._enum, ed, c);
   upb_strtable_insert(t, &e.e);
-
-  return true;
 }
 
-static bool insert_message(struct upb_strtable *t,
+static void insert_message(struct upb_strtable *t,
                            google_protobuf_DescriptorProto *d,
                            struct upb_string *base, bool sort,
-                           struct upb_context *c)
+                           struct upb_context *c,
+                           struct upb_status *status)
 {
-  if(!d->set_flags.has.name) return false;
+  if(!d->set_flags.has.name) {
+    upb_seterr(status, UPB_STATUS_ERROR,
+               "message in context '" UPB_STRFMT "' does not have a name",
+               UPB_STRARG(base));
+    return;
+  }
 
   /* We own this and must free it on destruct. */
   struct upb_string fqname = join(base, d->name);
 
-  /* Redefinition within a FileDescriptorProto is not allowed. */
-  if(upb_strtable_lookup(t, d->name)) {
+  if(upb_strtable_lookup(t, &fqname)) {
+    upb_seterr(status, UPB_STATUS_ERROR,
+               "attempted to redefine symbol '" UPB_STRFMT "'",
+               UPB_STRARG(&fqname));
     free(fqname.ptr);
-    return false;
+    return;
   }
 
   struct upb_symtab_entry e;
   e.e.key = fqname;
   e.type = UPB_SYM_MESSAGE;
   e.ref.msg = malloc(sizeof(*e.ref.msg));
-  if(!upb_msgdef_init(e.ref.msg, d, fqname, sort, c)) {
+  upb_msgdef_init(e.ref.msg, d, fqname, sort, c, status);
+  if(!upb_ok(status)) {
     free(fqname.ptr);
-    return false;
+    return;
   }
   upb_strtable_insert(t, &e.e);
 
   /* Add nested messages and enums. */
   if(d->set_flags.has.nested_type)
     for(unsigned int i = 0; i < d->nested_type->len; i++)
-      if(!insert_message(t, d->nested_type->elements[i], &fqname, sort, c))
-        return false;
+      insert_message(t, d->nested_type->elements[i], &fqname, sort, c, status);
 
   if(d->set_flags.has.enum_type)
     for(unsigned int i = 0; i < d->enum_type->len; i++)
-      if(!insert_enum(t, d->enum_type->elements[i], &fqname, c))
-        return false;
-
-  return true;
+      insert_enum(t, d->enum_type->elements[i], &fqname, c, status);
 }
 
-bool addfd(struct upb_strtable *addto, struct upb_strtable *existingdefs,
+void addfd(struct upb_strtable *addto, struct upb_strtable *existingdefs,
            google_protobuf_FileDescriptorProto *fd, bool sort,
-           struct upb_context *c)
+           struct upb_context *c, struct upb_status *status)
 {
   struct upb_string pkg = {.byte_len=0};
   if(fd->set_flags.has.package) pkg = *fd->package;
 
   if(fd->set_flags.has.message_type)
     for(unsigned int i = 0; i < fd->message_type->len; i++)
-      if(!insert_message(addto, fd->message_type->elements[i], &pkg, sort, c))
-        return false;
+      insert_message(addto, fd->message_type->elements[i], &pkg, sort, c, status);
 
   if(fd->set_flags.has.enum_type)
     for(unsigned int i = 0; i < fd->enum_type->len; i++)
-      if(!insert_enum(addto, fd->enum_type->elements[i], &pkg, c))
-        return false;
+      insert_enum(addto, fd->enum_type->elements[i], &pkg, c, status);
+
+  if(!upb_ok(status)) return;
 
   /* TODO: handle extensions and services. */
 
   /* Attempt to resolve all references. */
   struct upb_symtab_entry *e;
   for(e = upb_strtable_begin(addto); e; e = upb_strtable_next(addto, &e->e)) {
-    if(upb_strtable_lookup(existingdefs, &e->e.key))
-      return false;  /* Redefinition prohibited. */
+    if(upb_strtable_lookup(existingdefs, &e->e.key)) {
+      upb_seterr(status, UPB_STATUS_ERROR,
+                 "attempted to redefine symbol '" UPB_STRFMT "'",
+                 UPB_STRARG(&e->e.key));
+      return;
+    }
     if(e->type == UPB_SYM_MESSAGE) {
       struct upb_msgdef *m = e->ref.msg;
       for(unsigned int i = 0; i < m->num_fields; i++) {
@@ -285,16 +304,22 @@ bool addfd(struct upb_strtable *addto, struct upb_strtable *existingdefs,
                          UPB_SYM_ENUM);
         else
           continue;  /* No resolving necessary. */
-        if(!ref.msg) return false;  /* Ref. to undefined symbol. */
+        if(!ref.msg) {
+          upb_seterr(status, UPB_STATUS_ERROR,
+                     "could not resolve symbol '" UPB_STRFMT "'"
+                     " in context '" UPB_STRFMT "'",
+                     UPB_STRARG(fd->type_name), UPB_STRARG(&e->e.key));
+          return;
+        }
         upb_msgdef_setref(m, f, ref);
       }
     }
   }
-  return true;
 }
 
-bool upb_context_addfds(struct upb_context *c,
-                        google_protobuf_FileDescriptorSet *fds)
+void upb_context_addfds(struct upb_context *c,
+                        google_protobuf_FileDescriptorSet *fds,
+                        struct upb_status *status)
 {
   if(fds->set_flags.has.file) {
     /* Insert new symbols into a temporary table until we have verified that
@@ -303,11 +328,11 @@ bool upb_context_addfds(struct upb_context *c,
     upb_strtable_init(&tmp, 0, sizeof(struct upb_symtab_entry));
     upb_rwlock_rdlock(&c->lock);
     for(uint32_t i = 0; i < fds->file->len; i++) {
-      if(!addfd(&tmp, &c->symtab, fds->file->elements[i], true, c)) {
-        printf("Not added successfully!\n");
+      addfd(&tmp, &c->symtab, fds->file->elements[i], true, c, status);
+      if(!upb_ok(status)) {
         free_symtab(&tmp);
         upb_rwlock_unlock(&c->lock);
-        return false;
+        return;
       }
     }
     upb_rwlock_unlock(&c->lock);
@@ -322,13 +347,17 @@ bool upb_context_addfds(struct upb_context *c,
     }
     upb_strtable_free(&tmp);
   }
-  return true;
+  return;
 }
 
-bool upb_context_parsefds(struct upb_context *c, struct upb_string *fds_str) {
+void upb_context_parsefds(struct upb_context *c, struct upb_string *fds_str,
+                          struct upb_status *status)
+{
   struct upb_msg *fds = upb_msg_new(c->fds_msg);
-  if(upb_msg_parsestr(fds, fds_str->ptr, fds_str->byte_len) != UPB_STATUS_OK) return false;
-  if(!upb_context_addfds(c, (google_protobuf_FileDescriptorSet*)fds)) return false;
+  upb_msg_parsestr(fds, fds_str->ptr, fds_str->byte_len, status);
+  if(!upb_ok(status)) return;
+  upb_context_addfds(c, (google_protobuf_FileDescriptorSet*)fds, status);
+  if(!upb_ok(status)) return;
 
   {
     /* We own fds now, need to keep a ref so we can free it later. */
@@ -340,5 +369,5 @@ bool upb_context_parsefds(struct upb_context *c, struct upb_string *fds_str) {
     c->fds[c->fds_len++] = (google_protobuf_FileDescriptorSet*)fds;
     upb_rwlock_unlock(&c->lock);
   }
-  return true;
+  return;
 }

src/upb_context.h

@@ -102,13 +102,16 @@ void upb_context_enumerate(struct upb_context *c, upb_context_enumerator_t,
  * upb_context_addfd only returns true or false; it does not give any hint
  * about what happened in the case of failure.  This is because the descriptor
  * is expected to have been validated at the time it was parsed/generated. */
-bool upb_context_addfds(struct upb_context *c,
-                        struct google_protobuf_FileDescriptorSet *fds);
+void upb_context_addfds(struct upb_context *c,
+                        struct google_protobuf_FileDescriptorSet *fds,
+                        struct upb_status *status);
 
-bool upb_context_addfds(struct upb_context *c,
-                        struct google_protobuf_FileDescriptorSet *fds);
+void upb_context_addfds(struct upb_context *c,
+                        struct google_protobuf_FileDescriptorSet *fds,
+                        struct upb_status *status);
 
-bool upb_context_parsefds(struct upb_context *c, struct upb_string *fds);
+void upb_context_parsefds(struct upb_context *c, struct upb_string *fds,
+                          struct upb_status *status);
 
 #ifdef __cplusplus
 }  /* extern "C" */

src/upb_mm.c

@@ -9,14 +9,20 @@
 #include "upb_array.h"
 #include "upb_msg.h"
 
+static void upb_mm_destroy(union upb_value_ptr p, upb_mm_ptrtype type)
+{
+  if(*p.msg) {
+    union upb_mmptr mmptr = upb_mmptr_read(p, type);
+    upb_mm_unref(mmptr, type);
+  }
+}
+
 void upb_msg_destroy(struct upb_msg *msg) {
   uint32_t i;
   for(i = 0; i < msg->def->num_fields; i++) {
     struct upb_msg_fielddef *f = &msg->def->fields[i];
     if(!upb_msg_isset(msg, f) || !upb_field_ismm(f)) continue;
-    upb_mm_ptrtype type = upb_field_ptrtype(f);
-    union upb_mmptr mmptr = upb_mmptr_read(upb_msg_getptr(msg, f), type);
-    upb_mm_unref(mmptr, type);
+    upb_mm_destroy(upb_msg_getptr(msg, f), upb_field_ptrtype(f));
   }
   free(msg);
 }
@@ -26,11 +32,9 @@ void upb_array_destroy(struct upb_array *arr)
   if(upb_elem_ismm(arr->fielddef)) {
     upb_arraylen_t i;
     /* Unref elements. */
-    for(i = 0; i < arr->len; i++) {
+    for(i = 0; i < arr->size; i++) {
       union upb_value_ptr p = upb_array_getelementptr(arr, i);
-      upb_mm_ptrtype type = upb_elem_ptrtype(arr->fielddef);
-      union upb_mmptr mmptr = upb_mmptr_read(p, type);
-      upb_mm_unref(mmptr, type);
+      upb_mm_destroy(p, upb_elem_ptrtype(arr->fielddef));
     }
   }
   if(arr->size != 0) free(arr->elements._void);

src/upb_msg.c

@@ -42,9 +42,11 @@ void upb_msgdef_sortfds(google_protobuf_FieldDescriptorProto **fds, size_t num)
   qsort(fds, num, sizeof(void*), compare_fields);
 }
 
-bool upb_msgdef_init(struct upb_msgdef *m, google_protobuf_DescriptorProto *d,
-                     struct upb_string fqname, bool sort, struct upb_context *c)
+void upb_msgdef_init(struct upb_msgdef *m, google_protobuf_DescriptorProto *d,
+                     struct upb_string fqname, bool sort, struct upb_context *c,
+                     struct upb_status *status)
 {
+  (void)status;  // Nothing that can fail at the moment.
   int num_fields = d->set_flags.has.field ? d->field->len : 0;
   upb_inttable_init(&m->fields_by_num, num_fields,
                     sizeof(struct upb_fieldsbynum_entry));
@@ -97,8 +99,6 @@ bool upb_msgdef_init(struct upb_msgdef *m, google_protobuf_DescriptorProto *d,
 
   if(max_align > 0)
     m->size = ALIGN_UP(m->size, max_align);
-
-  return true;
 }
 
 void upb_msgdef_free(struct upb_msgdef *m)
@@ -125,26 +125,17 @@ void upb_msgdef_setref(struct upb_msgdef *m, struct upb_msg_fielddef *f,
 
 /* Parsing.  ******************************************************************/
 
-struct upb_msg_parser_frame {
+struct upb_msgparser_frame {
   struct upb_msg *msg;
 };
 
-struct upb_msg_parser {
+struct upb_msgparser {
   struct upb_cbparser *s;
   bool merge;
   bool byref;
-  struct upb_msg_parser_frame stack[UPB_MAX_NESTING], *top;
+  struct upb_msgparser_frame stack[UPB_MAX_NESTING], *top;
 };
 
-/* Parses protocol buffer data out of data which has length of len.  The data
- * need not be a complete protocol buffer.  The number of bytes parsed is
- * returned in *read, and the next call to upb_msg_parse must supply data that
- * is *read bytes past data in the logical stream. */
-upb_status_t upb_msg_parser_parse(struct upb_msg_parser *p,
-                                  void *data, size_t len, size_t *read);
-
-
-
 /* Helper function that returns a pointer to where the next value for field "f"
  * should be stored, taking into account whether f is an array that may need to
  * be allocated or resized. */
@@ -172,7 +163,7 @@ static union upb_value_ptr get_value_ptr(struct upb_msg *msg,
 static upb_field_type_t tag_cb(void *udata, struct upb_tag *tag,
                                void **user_field_desc)
 {
-  struct upb_msg_parser *mp = udata;
+  struct upb_msgparser *mp = udata;
   struct upb_msg_fielddef *f =
       upb_msg_fieldbynum(mp->top->msg->def, tag->field_number);
   if(!f || !upb_check_type(tag->wire_type, f->type))
@@ -181,23 +172,22 @@ static upb_field_type_t tag_cb(void *udata, struct upb_tag *tag,
   return f->type;
 }
 
-static upb_status_t value_cb(void *udata, uint8_t *buf, uint8_t *end,
-                             void *user_field_desc, uint8_t **outbuf)
+static void *value_cb(void *udata, uint8_t *buf, uint8_t *end,
+                      void *user_field_desc, struct upb_status *status)
 {
-  struct upb_msg_parser *mp = udata;
+  struct upb_msgparser *mp = udata;
   struct upb_msg_fielddef *f = user_field_desc;
   struct upb_msg *msg = mp->top->msg;
   union upb_value_ptr p = get_value_ptr(msg, f);
   upb_msg_set(msg, f);
-  UPB_CHECK(upb_parse_value(buf, end, f->type, p, outbuf));
-  return UPB_STATUS_OK;
+  return upb_parse_value(buf, end, f->type, p, status);
 }
 
 static void str_cb(void *udata, uint8_t *str,
                    size_t avail_len, size_t total_len,
                    void *udesc)
 {
-  struct upb_msg_parser *mp = udata;
+  struct upb_msgparser *mp = udata;
   struct upb_msg_fielddef *f = udesc;
   struct upb_msg *msg = mp->top->msg;
   union upb_value_ptr p = get_value_ptr(msg, f);
@@ -222,7 +212,7 @@ static void str_cb(void *udata, uint8_t *str,
 
 static void start_cb(void *udata, void *user_field_desc)
 {
-  struct upb_msg_parser *mp = udata;
+  struct upb_msgparser *mp = udata;
   struct upb_msg_fielddef *f = user_field_desc;
   struct upb_msg *oldmsg = mp->top->msg;
   union upb_value_ptr p = get_value_ptr(oldmsg, f);
@@ -243,53 +233,48 @@ static void start_cb(void *udata, void *user_field_desc)
 
 static void end_cb(void *udata)
 {
-  struct upb_msg_parser *mp = udata;
-  struct upb_msg *msg = mp->top->msg;
-  struct upb_msgdef *def = msg->def;
-
-  // Free any remaining dynamic storage we did not reuse.
-  for(uint32_t i = 0; i < def->num_fields; i++) {
-    struct upb_msg_fielddef *f = &def->fields[i];
-    union upb_value_ptr p = upb_msg_getptr(msg, f);
-    if(upb_msg_isset(msg, f) || !upb_field_ismm(f) || !*p.msg) continue;
-    union upb_mmptr mmptr = upb_mmptr_read(p, f->type);
-    upb_mm_unref(mmptr, f->type);
-  }
-
+  struct upb_msgparser *mp = udata;
   mp->top--;
 }
 
 /* Externally-visible functions for the msg parser. */
 
-void upb_msgparser_init(struct upb_msg_parser *s, struct upb_msg *msg, bool byref)
+struct upb_msgparser *upb_msgparser_new(struct upb_msgdef *def)
+{
+  (void)def;  // Not used atm.
+  struct upb_msgparser *mp = malloc(sizeof(struct upb_msgparser));
+  mp->s = upb_cbparser_new();
+  return mp;
+}
+
+void upb_msgparser_reset(struct upb_msgparser *s, struct upb_msg *msg, bool byref)
 {
-  s->s = upb_cbparser_new();
   upb_cbparser_reset(s->s, s, tag_cb, value_cb, str_cb, start_cb, end_cb);
   s->byref = byref;
   s->top = s->stack;
   s->top->msg = msg;
 }
 
-void upb_msgparser_free(struct upb_msg_parser *s)
+void upb_msgparser_free(struct upb_msgparser *s)
 {
   upb_cbparser_free(s->s);
+  free(s);
 }
 
-upb_status_t upb_msg_parsestr(struct upb_msg *msg, void *buf, size_t len)
+void upb_msg_parsestr(struct upb_msg *msg, void *buf, size_t len,
+                      struct upb_status *status)
 {
-  struct upb_msg_parser mp;
-  upb_msgparser_init(&mp, msg, false);
-  size_t read;
+  struct upb_msgparser *mp = upb_msgparser_new(msg->def);
+  upb_msgparser_reset(mp, msg, false);
   upb_msg_clear(msg);
-  upb_status_t ret = upb_msg_parser_parse(&mp, buf, len, &read);
-  upb_msgparser_free(&mp);
-  return ret;
+  upb_msgparser_parse(mp, buf, len, status);
+  upb_msgparser_free(mp);
 }
 
-upb_status_t upb_msg_parser_parse(struct upb_msg_parser *s,
-                                  void *data, size_t len, size_t *read)
+size_t upb_msgparser_parse(struct upb_msgparser *s, void *data, size_t len,
+                           struct upb_status *status)
 {
-  return upb_cbparser_parse(s->s, data, len, read);
+  return upb_cbparser_parse(s->s, data, len, status);
 }
 
 /* Serialization.  ************************************************************/
@@ -429,12 +414,13 @@ void upb_msg_serialize_init(struct upb_msg_serialize_state *s, struct upb_msg *m
   (void)sizes;
 }
 
-static upb_status_t serialize_tag(uint8_t *buf, uint8_t *end,
-                                  struct upb_msg_fielddef *f, uint8_t **outptr)
+#if 0
+static uint8_t *serialize_tag(uint8_t *buf, uint8_t *end,
+                              struct upb_msg_fielddef *f,
+                              struct upb_status *status)
 {
   /* TODO: need to have the field number also. */
-  UPB_CHECK(upb_put_UINT32(buf, end, f->type, outptr));
-  return UPB_STATUS_OK;
+  return upb_put_UINT32(buf, end, f->type, status);
 }
 
 /* Serializes the next set of bytes into buf (which has size len).  Returns
@@ -443,8 +429,8 @@ static upb_status_t serialize_tag(uint8_t *buf, uint8_t *end,
  *
  * The number of bytes written to buf is returned in *read.  This will be
  * equal to len unless we finished serializing. */
-upb_status_t upb_msg_serialize(struct upb_msg_serialize_state *s,
-                               void *_buf, size_t len, size_t *written)
+size_t upb_msg_serialize(struct upb_msg_serialize_state *s,
+                         void *_buf, size_t len, struct upb_status *status)
 {
   uint8_t *buf = _buf;
   uint8_t *end = buf + len;
@@ -456,18 +442,18 @@ upb_status_t upb_msg_serialize(struct upb_msg_serialize_state *s,
 
   while(buf < end) {
     struct upb_msg_fielddef *f = &m->fields[i];
-    union upb_value_ptr p = upb_msg_getptr(msg, f);
-    serialize_tag(buf, end, f, &buf);
+    //union upb_value_ptr p = upb_msg_getptr(msg, f);
+    buf = serialize_tag(buf, end, f, status);
     if(f->type == GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_MESSAGE) {
     } else if(f->type == GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_GROUP) {
     } else if(upb_isstring(f)) {
     } else {
-      upb_serialize_value(buf, end, f->type, p, &buf);
+      //upb_serialize_value(buf, end, f->type, p, status);
     }
   }
-  *written = buf - start;
-  return UPB_STATUS_OK;
+  return buf - start;
 }
+#endif
 
 
 /* Comparison.  ***************************************************************/

src/upb_msg.h

@@ -257,7 +257,17 @@ INLINE struct upb_msg_fielddef *upb_msg_fieldbyname(struct upb_msgdef *m,
 /* Parsing ********************************************************************/
 
 /* TODO: a stream parser. */
-upb_status_t upb_msg_parsestr(struct upb_msg *msg, void *buf, size_t len);
+void upb_msg_parsestr(struct upb_msg *msg, void *buf, size_t len,
+                      struct upb_status *status);
+
+struct upb_msgparser *upb_msgparser_new(struct upb_msgdef *def);
+void upb_msgparser_free(struct upb_msgparser *mp);
+
+void upb_msgparser_reset(struct upb_msgparser *mp, struct upb_msg *m,
+                         bool byref);
+
+size_t upb_msgparser_parse(struct upb_msgparser *mp, void *buf, size_t len,
+                           struct upb_status *status);
 
 /* Serialization  *************************************************************/
 
@@ -298,12 +308,11 @@ void upb_msg_serialize_init(struct upb_msg_serialize_state *s,
  *
  * The number of bytes written to buf is returned in *written.  This will be
  * equal to len unless we finished serializing. */
-upb_status_t upb_msg_serialize(struct upb_msg_serialize_state *s,
-                               void *buf, size_t len, size_t *written);
+size_t upb_msg_serialize(struct upb_msg_serialize_state *s,
+                         void *buf, size_t len, struct upb_status *status);
 
-upb_status_t upb_msg_serialize_all(struct upb_msg *msg,
-                                   struct upb_msgsizes *sizes,
-                                   void *buf);
+void upb_msg_serialize_all(struct upb_msg *msg, struct upb_msgsizes *sizes,
+                           void *buf, struct upb_status *status);
 
 /* Text dump  *****************************************************************/
 
@@ -327,10 +336,10 @@ void upb_msg_print(struct upb_msg *data, bool single_line, FILE *stream);
  * sort indicates whether or not it is safe to reorder the fields from the order
  * they appear in d.  This should be false if code has been compiled against a
  * header for this type that expects the given order. */
-bool upb_msgdef_init(struct upb_msgdef *m,
+void upb_msgdef_init(struct upb_msgdef *m,
                      struct google_protobuf_DescriptorProto *d,
                      struct upb_string fqname, bool sort,
-                     struct upb_context *c);
+                     struct upb_context *c, struct upb_status *status);
 void upb_msgdef_free(struct upb_msgdef *m);
 
 /* Sort the given field descriptors in-place, according to what we think is an

src/upb_parse.c

@@ -13,8 +13,8 @@
  * Parses a 64-bit varint that is known to be >= 2 bytes (the inline version
  * handles 1 and 2 byte varints).
  */
-upb_status_t upb_get_v_uint64_t_full(uint8_t *buf, uint8_t *end, uint64_t *val,
-                                     uint8_t **outbuf)
+uint8_t *upb_get_v_uint64_t_full(uint8_t *buf, uint8_t *end, uint64_t *val,
+                                 struct upb_status *status)
 {
   uint8_t *const maxend = buf + 10;
   uint8_t last = 0x80;
@@ -24,27 +24,33 @@ upb_status_t upb_get_v_uint64_t_full(uint8_t *buf, uint8_t *end, uint64_t *val,
   for(bitpos = 0; buf < (uint8_t*)end && (last & 0x80); buf++, bitpos += 7)
     *val |= ((uint64_t)((last = *buf) & 0x7F)) << bitpos;
 
-  if(buf >= end && buf <= maxend && (last & 0x80))
-    return UPB_STATUS_NEED_MORE_DATA;
-  if(buf > maxend)
-    return UPB_ERROR_UNTERMINATED_VARINT;
+  if(buf >= end && buf <= maxend && (last & 0x80)) {
+    upb_seterr(status, UPB_STATUS_NEED_MORE_DATA,
+               "Provided data ended in the middle of a varint.\n");
+    buf = end;
+  } else if(buf > maxend) {
+    upb_seterr(status, UPB_ERROR_UNTERMINATED_VARINT,
+               "Varint was unterminated after 10 bytes.\n");
+    buf = end;
+  }
 
-  *outbuf = buf;
-  return UPB_STATUS_OK;
+  return buf;
 }
 
-upb_status_t upb_parse_wire_value(uint8_t *buf, uint8_t *end, upb_wire_type_t wt,
-                                  union upb_wire_value *wv, uint8_t **outbuf)
+uint8_t *upb_parse_wire_value(uint8_t *buf, uint8_t *end, upb_wire_type_t wt,
+                              union upb_wire_value *wv,
+                              struct upb_status *status)
 {
   switch(wt) {
     case UPB_WIRE_TYPE_VARINT:
-      return upb_get_v_uint64_t(buf, end, &wv->varint, outbuf);
+      return upb_get_v_uint64_t(buf, end, &wv->varint, status);
     case UPB_WIRE_TYPE_64BIT:
-      return upb_get_f_uint64_t(buf, end, &wv->_64bit, outbuf);
+      return upb_get_f_uint64_t(buf, end, &wv->_64bit, status);
     case UPB_WIRE_TYPE_32BIT:
-      return upb_get_f_uint32_t(buf, end, &wv->_32bit, outbuf);
+      return upb_get_f_uint32_t(buf, end, &wv->_32bit, status);
     default:
-      return UPB_ERROR_ILLEGAL;  // Doesn't handle delimited, groups.
+      status->code = UPB_STATUS_ERROR;  // Doesn't handle delimited, groups.
+      return end;
   }
 }
 
@@ -52,30 +58,31 @@ upb_status_t upb_parse_wire_value(uint8_t *buf, uint8_t *end, upb_wire_type_t wt
  * Advances buf past the current wire value (of type wt), saving the result in
  * outbuf.
  */
-static upb_status_t skip_wire_value(uint8_t *buf, uint8_t *end, upb_wire_type_t wt,
-                                    uint8_t **outbuf)
+static uint8_t *skip_wire_value(uint8_t *buf, uint8_t *end, upb_wire_type_t wt,
+                                struct upb_status *status)
 {
   switch(wt) {
     case UPB_WIRE_TYPE_VARINT:
-      return upb_skip_v_uint64_t(buf, end, outbuf);
+      return upb_skip_v_uint64_t(buf, end, status);
     case UPB_WIRE_TYPE_64BIT:
-      return upb_skip_f_uint64_t(buf, end, outbuf);
+      return upb_skip_f_uint64_t(buf, end, status);
     case UPB_WIRE_TYPE_32BIT:
-      return upb_skip_f_uint32_t(buf, end, outbuf);
+      return upb_skip_f_uint32_t(buf, end, status);
     case UPB_WIRE_TYPE_START_GROUP:
       // TODO: skip to matching end group.
     case UPB_WIRE_TYPE_END_GROUP:
-      return UPB_STATUS_OK;
+      return buf;
     default:
-      return UPB_ERROR_ILLEGAL;
+      status->code = UPB_STATUS_ERROR;
+      return end;
   }
 }
 
-upb_status_t upb_parse_value(uint8_t *buf, uint8_t *end, upb_field_type_t ft,
-                             union upb_value_ptr v, uint8_t **outbuf)
+uint8_t *upb_parse_value(uint8_t *buf, uint8_t *end, upb_field_type_t ft,
+                         union upb_value_ptr v, struct upb_status *status)
 {
 #define CASE(t, member_name) \
-  case UPB_TYPENUM(t): return upb_get_ ## t(buf, end, v.member_name, outbuf);
+  case UPB_TYPENUM(t): return upb_get_ ## t(buf, end, v.member_name, status);
 
   switch(ft) {
     CASE(DOUBLE,   _double)
@@ -92,7 +99,7 @@ upb_status_t upb_parse_value(uint8_t *buf, uint8_t *end, upb_field_type_t ft,
     CASE(SFIXED64, int64)
     CASE(BOOL,     _bool)
     CASE(ENUM,     int32)
-    default: return UPB_ERROR_ILLEGAL;
+    default: return end;
   }
 
 #undef CASE
@@ -146,23 +153,26 @@ void upb_cbparser_reset(struct upb_cbparser *p, void *udata,
  * Pushes a new stack frame for a submessage with the given len (which will
  * be zero if the submessage is a group).
  */
-static upb_status_t push(struct upb_cbparser *s, uint8_t *start,
-                         uint32_t submsg_len, void *user_field_desc,
-                         uint8_t **submsg_end)
+static uint8_t *push(struct upb_cbparser *s, uint8_t *start,
+                     uint32_t submsg_len, void *user_field_desc,
+                     struct upb_status *status)
 {
   s->top++;
-  if(s->top >= s->limit)
-    return UPB_ERROR_STACK_OVERFLOW;
+  if(s->top >= s->limit) {
+    upb_seterr(status, UPB_STATUS_ERROR,
+               "Nesting exceeded maximum (%d levels)\n",
+               UPB_MAX_NESTING);
+    return NULL;
+  }
   *s->top = s->completed_offset + submsg_len;
 
   if(s->start_cb)
     s->start_cb(s->udata, user_field_desc);
 
   if(*s->top > 0)
-    *submsg_end = start + (*s->top - s->completed_offset);
+    return start + (*s->top - s->completed_offset);
   else
-    *submsg_end = (void*)UINTPTR_MAX;
-  return UPB_STATUS_OK;
+    return (void*)UINTPTR_MAX;
 }
 
 /**
@@ -183,15 +193,14 @@ static void *pop(struct upb_cbparser *s, uint8_t *start)
 }
 
 
-upb_status_t upb_cbparser_parse(struct upb_cbparser *s, void *_buf, size_t len,
-                                size_t *read)
+size_t upb_cbparser_parse(struct upb_cbparser *s, void *_buf, size_t len,
+                          struct upb_status *status)
 {
   uint8_t *buf = _buf;
   uint8_t *completed = buf;
   uint8_t *const start = buf;  // ptr equivalent of s->completed_offset
   uint8_t *end = buf + len;
   uint8_t *submsg_end = *s->top > 0 ? buf + *s->top : (uint8_t*)UINTPTR_MAX;
-  upb_status_t status = UPB_STATUS_OK;
 
   // Make local copies so optimizer knows they won't change.
   upb_tag_cb tag_cb = s->tag_cb;
@@ -199,13 +208,14 @@ upb_status_t upb_cbparser_parse(struct upb_cbparser *s, void *_buf, size_t len,
   upb_value_cb value_cb = s->value_cb;
   void *udata = s->udata;
 
-#define CHECK(exp) do { if((status = exp) != UPB_STATUS_OK) goto err; } while(0)
+#define CHECK_STATUS() do { if(!upb_ok(status)) goto err; } while(0)
 
   // Main loop: parse a tag, then handle the value.
   while(buf < end) {
     struct upb_tag tag;
-    CHECK(parse_tag(buf, end, &tag, &buf));
+    buf = parse_tag(buf, end, &tag, status);
     if(tag.wire_type == UPB_WIRE_TYPE_END_GROUP) {
+      CHECK_STATUS();
       submsg_end = pop(s, start);
       completed = buf;
       continue;
@@ -215,10 +225,11 @@ upb_status_t upb_cbparser_parse(struct upb_cbparser *s, void *_buf, size_t len,
     upb_field_type_t ft = tag_cb(udata, &tag, &udesc);
     if(tag.wire_type == UPB_WIRE_TYPE_DELIMITED) {
       int32_t delim_len;
-      CHECK(upb_get_INT32(buf, end, &delim_len, &buf));
+      buf = upb_get_INT32(buf, end, &delim_len, status);
+      CHECK_STATUS();
       uint8_t *delim_end = buf + delim_len;
       if(ft == UPB_TYPENUM(MESSAGE)) {
-        CHECK(push(s, start, delim_end - start, udesc, &submsg_end));
+        submsg_end = push(s, start, delim_end - start, udesc, status);
       } else {
         if(upb_isstringtype(ft)) {
           size_t avail_len = UPB_MIN(delim_end, end) - buf;
@@ -230,20 +241,21 @@ upb_status_t upb_cbparser_parse(struct upb_cbparser *s, void *_buf, size_t len,
       // Scalar (non-delimited) value.
       switch(ft) {
         case 0:  // Client elected to skip.
-          CHECK(skip_wire_value(buf, end, tag.wire_type, &buf));
+          buf = skip_wire_value(buf, end, tag.wire_type, status);
           break;
         case UPB_TYPENUM(GROUP):
-          CHECK(push(s, start, 0, udesc, &submsg_end));
+          submsg_end = push(s, start, 0, udesc, status);
           break;
         default:
-          CHECK(value_cb(udata, buf, end, udesc, &buf));
+          buf = value_cb(udata, buf, end, udesc, status);
           break;
       }
     }
+    CHECK_STATUS();
 
     while(buf >= submsg_end) {
       if(buf > submsg_end) {
-        return UPB_ERROR_BAD_SUBMESSAGE_END;
+        return UPB_STATUS_ERROR;  // Bad submessage end.
       }
       submsg_end = pop(s, start);
     }
@@ -251,8 +263,9 @@ upb_status_t upb_cbparser_parse(struct upb_cbparser *s, void *_buf, size_t len,
     completed = buf;
   }
 
+  size_t read;
 err:
-  *read = (char*)completed - (char*)start;
-  s->completed_offset += *read;
-  return status;
+  read = (char*)completed - (char*)start;
+  s->completed_offset += read;
+  return read;
 }

src/upb_parse.h

@@ -45,8 +45,8 @@ typedef upb_field_type_t (*upb_tag_cb)(void *udata, struct upb_tag *tag,
 //
 // Note that this callback can be called several times in a row for a single
 // call to tag_cb in the case of packed arrays.
-typedef upb_status_t (*upb_value_cb)(void *udata, uint8_t *buf, uint8_t *end,
-                                     void *user_field_desc, uint8_t **outbuf);
+typedef void *(*upb_value_cb)(void *udata, uint8_t *buf, uint8_t *end,
+                              void *user_field_desc, struct upb_status *status);
 
 // The string callback is called when a string is parsed.  avail_len is the
 // number of bytes that are currently available at str.  If the client is
@@ -96,8 +96,8 @@ void upb_cbparser_reset(struct upb_cbparser *p, void *udata,
 //
 // TODO: see if we can provide the following guarantee efficiently:
 //   *read will always be >= len. */
-upb_status_t upb_cbparser_parse(struct upb_cbparser *p, void *buf, size_t len,
-                                size_t *read);
+size_t upb_cbparser_parse(struct upb_cbparser *p, void *buf, size_t len,
+                          struct upb_status *status);
 
 extern upb_wire_type_t upb_expected_wire_types[];
 // Returns true if wt is the correct on-the-wire type for ft.
@@ -109,56 +109,59 @@ INLINE bool upb_check_type(upb_wire_type_t wt, upb_field_type_t ft) {
 /* Data-consuming functions (to be called from value cb). *********************/
 
 // Parses and converts a value from the character data starting at buf (but not
-// past end).  *outbuf will be set to one past the data that was read.  The
+// past end).  Returns a pointer that is one past the data that was read.  The
 // caller must have previously checked that the wire type is appropriate for
 // this field type.
-upb_status_t upb_parse_value(uint8_t *buf, uint8_t *end, upb_field_type_t ft,
-                             union upb_value_ptr v, uint8_t **outbuf);
+uint8_t *upb_parse_value(uint8_t *buf, uint8_t *end, upb_field_type_t ft,
+                         union upb_value_ptr v, struct upb_status *status);
 
 // Parses a wire value with the given type (which must have been obtained from
-// a tag that was just parsed) and sets *outbuf to one past the data that was
-// read.
-upb_status_t upb_parse_wire_value(uint8_t *buf, uint8_t *end, upb_wire_type_t wt,
-                                  union upb_wire_value *wv, uint8_t **outbuf);
+// a tag that was just parsed) and returns a pointer to one past the data that
+// was read.
+uint8_t *upb_parse_wire_value(uint8_t *buf, uint8_t *end, upb_wire_type_t wt,
+                              union upb_wire_value *wv,
+                              struct upb_status *status);
 
 /* Functions to read wire values. *********************************************/
 
 // Most clients will not want to use these directly.
 
-upb_status_t upb_get_v_uint64_t_full(uint8_t *buf, uint8_t *end, uint64_t *val,
-                                     uint8_t **outbuf);
+uint8_t *upb_get_v_uint64_t_full(uint8_t *buf, uint8_t *end, uint64_t *val,
+                                 struct upb_status *status);
 
 // Gets a varint (wire type: UPB_WIRE_TYPE_VARINT).
-INLINE upb_status_t upb_get_v_uint64_t(uint8_t *buf, uint8_t *end, uint64_t *val,
-                                       uint8_t **outbuf)
+INLINE uint8_t *upb_get_v_uint64_t(uint8_t *buf, uint8_t *end, uint64_t *val,
+                                   struct upb_status *status)
 {
   // We inline this common case (1-byte varints), if that fails we dispatch to
   // the full (non-inlined) version.
   if((*buf & 0x80) == 0) {
     *val = *buf & 0x7f;
-    *outbuf = buf + 1;
-    return UPB_STATUS_OK;
+    return buf + 1;
   } else {
-    return upb_get_v_uint64_t_full(buf, end, val, outbuf);
+    return upb_get_v_uint64_t_full(buf, end, val, status);
   }
 }
 
 // Gets a varint -- called when we only need 32 bits of it.
-INLINE upb_status_t upb_get_v_uint32_t(uint8_t *buf, uint8_t *end,
-                                       uint32_t *val, uint8_t **outbuf)
+INLINE uint8_t *upb_get_v_uint32_t(uint8_t *buf, uint8_t *end,
+                                   uint32_t *val, struct upb_status *status)
 {
   uint64_t val64;
-  UPB_CHECK(upb_get_v_uint64_t(buf, end, &val64, outbuf));
+  uint8_t *ret = upb_get_v_uint64_t(buf, end, &val64, status);
   *val = (uint32_t)val64;  // Discard the high bits.
-  return UPB_STATUS_OK;
+  return ret;
 }
 
 // Gets a fixed-length 32-bit integer (wire type: UPB_WIRE_TYPE_32BIT).
-INLINE upb_status_t upb_get_f_uint32_t(uint8_t *buf, uint8_t *end,
-                                       uint32_t *val, uint8_t **outbuf)
+INLINE uint8_t *upb_get_f_uint32_t(uint8_t *buf, uint8_t *end,
+                                   uint32_t *val, struct upb_status *status)
 {
   uint8_t *uint32_end = buf + sizeof(uint32_t);
-  if(uint32_end > end) return UPB_STATUS_NEED_MORE_DATA;
+  if(uint32_end > end) {
+    status->code = UPB_STATUS_NEED_MORE_DATA;
+    return end;
+  }
 #if UPB_UNALIGNED_READS_OK
   *val = *(uint32_t*)buf;
 #else
@@ -166,16 +169,18 @@ INLINE upb_status_t upb_get_f_uint32_t(uint8_t *buf, uint8_t *end,
   *val = SHL(buf[0], 0) | SHL(buf[1], 8) | SHL(buf[2], 16) | SHL(buf[3], 24);
 #undef SHL
 #endif
-  *outbuf = uint32_end;
-  return UPB_STATUS_OK;
+  return uint32_end;
 }
 
 // Gets a fixed-length 64-bit integer (wire type: UPB_WIRE_TYPE_64BIT).
-INLINE upb_status_t upb_get_f_uint64_t(uint8_t *buf, uint8_t *end,
-                                       uint64_t *val, uint8_t **outbuf)
+INLINE uint8_t *upb_get_f_uint64_t(uint8_t *buf, uint8_t *end,
+                                   uint64_t *val, struct upb_status *status)
 {
   uint8_t *uint64_end = buf + sizeof(uint64_t);
-  if(uint64_end > end) return UPB_STATUS_NEED_MORE_DATA;
+  if(uint64_end > end) {
+    status->code = UPB_STATUS_NEED_MORE_DATA;
+    return end;
+  }
 #if UPB_UNALIGNED_READS_OK
   *val = *(uint64_t*)buf;
 #else
@@ -184,39 +189,47 @@ INLINE upb_status_t upb_get_f_uint64_t(uint8_t *buf, uint8_t *end,
          SHL(buf[4], 32) | SHL(buf[5], 40) | SHL(buf[6], 48) | SHL(buf[7], 56);
 #undef SHL
 #endif
-  *outbuf = uint64_end;
-  return UPB_STATUS_OK;
+  return uint64_end;
 }
 
-INLINE upb_status_t upb_skip_v_uint64_t(uint8_t *buf, uint8_t *end,
-                                        uint8_t **outbuf)
+INLINE uint8_t *upb_skip_v_uint64_t(uint8_t *buf, uint8_t *end,
+                                    struct upb_status *status)
 {
   uint8_t *const maxend = buf + 10;
   uint8_t last = 0x80;
   for(; buf < (uint8_t*)end && (last & 0x80); buf++)
     last = *buf;
-  if(buf >= end && buf <= maxend && (last & 0x80)) return UPB_STATUS_NEED_MORE_DATA;
-  if(buf > maxend) return UPB_ERROR_UNTERMINATED_VARINT;
-  *outbuf = buf;
-  return UPB_STATUS_OK;
+
+  if(buf >= end && buf <= maxend && (last & 0x80)) {
+    status->code = UPB_STATUS_NEED_MORE_DATA;
+    buf = end;
+  } else if(buf > maxend) {
+    status->code = UPB_ERROR_UNTERMINATED_VARINT;
+    buf = end;
+  }
+  return buf;
 }
 
-INLINE upb_status_t upb_skip_f_uint32_t(uint8_t *buf, uint8_t *end,
-                                        uint8_t **outbuf)
+INLINE uint8_t *upb_skip_f_uint32_t(uint8_t *buf, uint8_t *end,
+                                    struct upb_status *status)
 {
   uint8_t *uint32_end = buf + sizeof(uint32_t);
-  if(uint32_end > end) return UPB_STATUS_NEED_MORE_DATA;
-  *outbuf = uint32_end;
-  return UPB_STATUS_OK;
+  if(uint32_end > end) {
+    status->code = UPB_STATUS_NEED_MORE_DATA;
+    return end;
+  }
+  return uint32_end;
 }
 
-INLINE upb_status_t upb_skip_f_uint64_t(uint8_t *buf, uint8_t *end,
-                                        uint8_t **outbuf)
+INLINE uint8_t *upb_skip_f_uint64_t(uint8_t *buf, uint8_t *end,
+                                    struct upb_status *status)
 {
   uint8_t *uint64_end = buf + sizeof(uint64_t);
-  if(uint64_end > end) return UPB_STATUS_NEED_MORE_DATA;
-  *outbuf = uint64_end;
-  return UPB_STATUS_OK;
+  if(uint64_end > end) {
+    status->code = UPB_STATUS_NEED_MORE_DATA;
+    return end;
+  }
+  return uint64_end;
 }
 
 
@@ -232,9 +245,10 @@ INLINE int64_t upb_zzdec_64(uint64_t n) { return (n >> 1) ^ -(int64_t)(n & 1); }
 //  // Reads and converts a .proto value from buf, placing it in d.
 //  // "end" indicates the end of the current buffer (if the buffer does
 //  // not contain the entire value UPB_STATUS_NEED_MORE_DATA is returned).
-//  // On success, *outbuf will point to the first byte that was not consumed.
-//  upb_status_t upb_get_INT32(uint8_t *buf, uint8_t *end, int32_t *d,
-//                             uint8_t **outbuf);
+//  // On success, a pointer will be returned to the first byte that was
+//  // not consumed.
+//  uint8_t *upb_get_INT32(uint8_t *buf, uint8_t *end, int32_t *d,
+//                         struct upb_status *status);
 //
 //  // Given an already read wire value s (source), convert it to a .proto
 //  // value and return it.
@@ -247,12 +261,12 @@ INLINE int64_t upb_zzdec_64(uint64_t n) { return (n >> 1) ^ -(int64_t)(n & 1); }
   INLINE val_t upb_wvtov_ ## type(wire_t s)
 
 #define GET(type, v_or_f, wire_t, val_t, member_name) \
-  INLINE upb_status_t upb_get_ ## type(uint8_t *buf, uint8_t *end, val_t *d, \
-                                       uint8_t **outbuf) { \
-    wire_t tmp; \
-    UPB_CHECK(upb_get_ ## v_or_f ## _ ## wire_t(buf, end, &tmp, outbuf)); \
+  INLINE uint8_t *upb_get_ ## type(uint8_t *buf, uint8_t *end, val_t *d, \
+                                   struct upb_status *status) { \
+    wire_t tmp = 0; \
+    uint8_t *ret = upb_get_ ## v_or_f ## _ ## wire_t(buf, end, &tmp, status); \
     *d = upb_wvtov_ ## type(tmp); \
-    return UPB_STATUS_OK; \
+    return ret; \
   }
 
 #define T(type, v_or_f, wire_t, val_t, member_name) \
@@ -288,14 +302,14 @@ T(FLOAT,    f, uint32_t, float,    _float)  {
 #undef T
 
 // Parses a tag, places the result in *tag.
-INLINE upb_status_t parse_tag(uint8_t *buf, uint8_t *end, struct upb_tag *tag,
-                              uint8_t **outbuf)
+INLINE uint8_t *parse_tag(uint8_t *buf, uint8_t *end, struct upb_tag *tag,
+                          struct upb_status *status)
 {
   uint32_t tag_int;
-  UPB_CHECK(upb_get_v_uint32_t(buf, end, &tag_int, outbuf));
+  uint8_t *ret = upb_get_v_uint32_t(buf, end, &tag_int, status);
   tag->wire_type    = (upb_wire_type_t)(tag_int & 0x07);
   tag->field_number = tag_int >> 3;
-  return UPB_STATUS_OK;
+  return ret;
 }
 
 #ifdef __cplusplus

src/upb_serialize.h

@@ -30,33 +30,35 @@ extern "C" {
 /* Functions to write wire values. ********************************************/
 
 /* Puts a varint (wire type: UPB_WIRE_TYPE_VARINT). */
-INLINE upb_status_t upb_put_v_uint64_t(uint8_t *buf, uint8_t *end, uint64_t val,
-                                       uint8_t **outbuf)
+INLINE uint8_t *upb_put_v_uint64_t(uint8_t *buf, uint8_t *end, uint64_t val,
+                                   struct upb_status *status)
 {
   do {
     uint8_t byte = val & 0x7f;
     val >>= 7;
     if(val) byte |= 0x80;
-    if(buf >= end) return UPB_STATUS_NEED_MORE_DATA;
+    if(buf >= end) {
+      status->code = UPB_STATUS_NEED_MORE_DATA;
+      return end;
+    }
     *buf++ = byte;
   } while(val);
-  *outbuf = buf;
-  return UPB_STATUS_OK;
+  return buf;
 }
 
 /* Puts an unsigned 32-bit varint, verbatim.  Never uses the high 64 bits. */
-INLINE upb_status_t upb_put_v_uint32_t(uint8_t *buf, uint8_t *end,
-                                       uint32_t val, uint8_t **outbuf)
+INLINE uint8_t *upb_put_v_uint32_t(uint8_t *buf, uint8_t *end,
+                                   uint32_t val, struct upb_status *status)
 {
-  return upb_put_v_uint64_t(buf, end, val, outbuf);
+  return upb_put_v_uint64_t(buf, end, val, status);
 }
 
 /* Puts a signed 32-bit varint, first sign-extending to 64-bits.  We do this to
  * maintain wire-compatibility with 64-bit signed integers. */
-INLINE upb_status_t upb_put_v_int32_t(uint8_t *buf, uint8_t *end,
-                                      int32_t val, uint8_t **outbuf)
+INLINE uint8_t *upb_put_v_int32_t(uint8_t *buf, uint8_t *end,
+                                  int32_t val, struct upb_status *status)
 {
-  return upb_put_v_uint64_t(buf, end, (int64_t)val, outbuf);
+  return upb_put_v_uint64_t(buf, end, (int64_t)val, status);
 }
 
 INLINE void upb_put32(uint8_t *buf, uint32_t val) {
@@ -67,34 +69,38 @@ INLINE void upb_put32(uint8_t *buf, uint32_t val) {
 }
 
 /* Puts a fixed-length 32-bit integer (wire type: UPB_WIRE_TYPE_32BIT). */
-INLINE upb_status_t upb_put_f_uint32_t(uint8_t *buf, uint8_t *end,
-                                       uint32_t val, uint8_t **outbuf)
+INLINE uint8_t *upb_put_f_uint32_t(uint8_t *buf, uint8_t *end,
+                                   uint32_t val, struct upb_status *status)
 {
   uint8_t *uint32_end = buf + sizeof(uint32_t);
-  if(uint32_end > end) return UPB_STATUS_NEED_MORE_DATA;
+  if(uint32_end > end) {
+    status->code = UPB_STATUS_NEED_MORE_DATA;
+    return end;
+  }
 #if UPB_UNALIGNED_READS_OK
   *(uint32_t*)buf = val;
 #else
   upb_put32(buf, val);
 #endif
-  *outbuf = uint32_end;
-  return UPB_STATUS_OK;
+  return uint32_end;
 }
 
 /* Puts a fixed-length 64-bit integer (wire type: UPB_WIRE_TYPE_64BIT). */
-INLINE upb_status_t upb_put_f_uint64_t(uint8_t *buf, uint8_t *end,
-                                       uint64_t val, uint8_t **outbuf)
+INLINE uint8_t *upb_put_f_uint64_t(uint8_t *buf, uint8_t *end,
+                                   uint64_t val, struct upb_status *status)
 {
   uint8_t *uint64_end = buf + sizeof(uint64_t);
-  if(uint64_end > end) return UPB_STATUS_NEED_MORE_DATA;
+  if(uint64_end > end) {
+    status->code = UPB_STATUS_NEED_MORE_DATA;
+    return end;
+  }
 #if UPB_UNALIGNED_READS_OK
   *(uint64_t*)buf = val;
 #else
   upb_put32(buf, (uint32_t)val);
   upb_put32(buf, (uint32_t)(val >> 32));
 #endif
-  *outbuf = uint64_end;
-  return UPB_STATUS_OK;
+  return uint64_end;
 }
 
 INLINE size_t upb_v_uint64_t_size(uint64_t val) {
@@ -136,8 +142,8 @@ INLINE uint64_t upb_zzenc_64(int64_t n) { return (n << 1) ^ (n >> 63); }
  *  // of the current available buffer (if the buffer does not contain enough
  *  // space UPB_STATUS_NEED_MORE_DATA is returned).  On success, *outbuf will
  *  // point one past the data that was written.
- *  upb_status_t upb_put_INT32(uint8_t *buf, uint8_t *end, int32_t val,
- *                             uint8_t **outbuf);
+ *  uint8_t *upb_put_INT32(uint8_t *buf, uint8_t *end, int32_t val,
+ *                         struct upb_status *status);
  *
  *  // Returns the number of bytes required to serialize val.
  *  size_t upb_get_INT32_size(int32_t val);
@@ -150,11 +156,10 @@ INLINE uint64_t upb_zzenc_64(int64_t n) { return (n << 1) ^ (n >> 63); }
   INLINE wire_t upb_vtowv_ ## type(val_t s)
 
 #define PUT(type, v_or_f, wire_t, val_t, member_name) \
-  INLINE upb_status_t upb_put_ ## type(uint8_t *buf, uint8_t *end, val_t val, \
-                                       uint8_t **outbuf) { \
+  INLINE uint8_t *upb_put_ ## type(uint8_t *buf, uint8_t *end, val_t val, \
+                                   struct upb_status *status) { \
     wire_t tmp = upb_vtowv_ ## type(val); \
-    UPB_CHECK(upb_put_ ## v_or_f ## _ ## wire_t(buf, end, tmp, outbuf)); \
-    return UPB_STATUS_OK; \
+    return upb_put_ ## v_or_f ## _ ## wire_t(buf, end, tmp, status); \
   }
 
 #define T(type, v_or_f, wire_t, val_t, member_name) \
@@ -195,8 +200,8 @@ INLINE size_t upb_get_tag_size(uint32_t fieldnum) {
   return upb_v_uint64_t_size((uint64_t)fieldnum << 3);
 }
 
-upb_status_t upb_serialize_value(uint8_t *buf, uint8_t *end, upb_field_type_t ft,
-                                 union upb_value_ptr v, uint8_t **outbuf);
+uint8_t *upb_serialize_value(uint8_t *buf, uint8_t *end, upb_field_type_t ft,
+                             union upb_value_ptr v, struct upb_status *status);
 
 #ifdef __cplusplus
 }  /* extern "C" */

tests/test_vs_proto2.cc

@@ -168,7 +168,9 @@ void parse_and_compare(MESSAGE_CIDENT *proto2_msg, struct upb_msg *upb_msg,
 {
   // Parse to both proto2 and upb.
   ASSERT(proto2_msg->ParseFromArray(str->ptr, str->byte_len));
-  ASSERT(upb_msg_parsestr(upb_msg, str->ptr, str->byte_len) == UPB_STATUS_OK);
+  struct upb_status status = UPB_STATUS_INIT;
+  upb_msg_parsestr(upb_msg, str->ptr, str->byte_len, &status);
+  ASSERT(upb_ok(&status));
   compare(*proto2_msg, upb_msg);
 }
 
@@ -189,14 +191,17 @@ int main(int argc, char *argv[])
   }
 
   // Initialize upb state, parse descriptor.
+  struct upb_status status = UPB_STATUS_INIT;
   struct upb_context *c = upb_context_new();
   struct upb_string *fds = upb_strreadfile(MESSAGE_DESCRIPTOR_FILE);
   if(!fds) {
     fprintf(stderr, "Couldn't read " MESSAGE_DESCRIPTOR_FILE ".\n");
     return 1;
   }
-  if(!upb_context_parsefds(c, fds)) {
-    fprintf(stderr, "Error importing " MESSAGE_DESCRIPTOR_FILE ".\n");
+  upb_context_parsefds(c, fds, &status);
+  if(!upb_ok(&status)) {
+    fprintf(stderr, "Error importing " MESSAGE_DESCRIPTOR_FILE ": %s.\n",
+            status.msg);
     return 1;
   }
   upb_string_unref(fds);

tests/tests.c

@@ -15,18 +15,18 @@ int num_assertions = 0;
 static void test_get_v_uint64_t()
 {
 #define TEST(name, bytes, val) {\
-    upb_status_t status; \
+    struct upb_status status = UPB_STATUS_INIT; \
     uint8_t name[] = bytes; \
     uint8_t *name ## _buf = name; \
     uint64_t name ## _val = 0; \
-    status = upb_get_v_uint64_t(name, name + sizeof(name) - 1, &name ## _val, &name ## _buf); \
-    ASSERT(status == UPB_STATUS_OK); \
+    name ## _buf = upb_get_v_uint64_t(name, name + sizeof(name) - 1, &name ## _val, &status); \
+    ASSERT(upb_ok(&status)); \
     ASSERT(name ## _val == val); \
     ASSERT(name ## _buf == name + sizeof(name) - 1);  /* - 1 for NULL */ \
     /* Test NEED_MORE_DATA. */ \
     if(sizeof(name) > 2) { \
-      status = upb_get_v_uint64_t(name, name + sizeof(name) - 2, &name ## _val, &name ## _buf); \
-      ASSERT(status == UPB_STATUS_NEED_MORE_DATA); \
+      name ## _buf = upb_get_v_uint64_t(name, name + sizeof(name) - 2, &name ## _val, &status); \
+      ASSERT(status.code == UPB_STATUS_NEED_MORE_DATA); \
     } \
   }
 
@@ -44,18 +44,18 @@ static void test_get_v_uint64_t()
 #undef TEST
 
   uint8_t twelvebyte[] = {0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01, 0x01};
-  uint8_t *twelvebyte_buf = twelvebyte;
   uint64_t twelvebyte_val = 0;
-  upb_status_t status;
+  struct upb_status status = UPB_STATUS_INIT;
   /* A varint that terminates before hitting the end of the provided buffer,
    * but in too many bytes (11 instead of 10). */
-  status = upb_get_v_uint64_t(twelvebyte_buf, twelvebyte + 12, &twelvebyte_val, &twelvebyte_buf);
-  ASSERT(status == UPB_ERROR_UNTERMINATED_VARINT);
+  upb_get_v_uint64_t(twelvebyte, twelvebyte + 12, &twelvebyte_val, &status);
+  ASSERT(status.code == UPB_ERROR_UNTERMINATED_VARINT);
 
   /* A varint that terminates simultaneously with the end of the provided
    * buffer, but in too many bytes (11 instead of 10). */
-  status = upb_get_v_uint64_t(twelvebyte_buf, twelvebyte + 11, &twelvebyte_val, &twelvebyte_buf);
-  ASSERT(status == UPB_ERROR_UNTERMINATED_VARINT);
+  upb_reset(&status);
+  upb_get_v_uint64_t(twelvebyte, twelvebyte + 11, &twelvebyte_val, &status);
+  ASSERT(status.code == UPB_ERROR_UNTERMINATED_VARINT);
 
   /* A varint whose buffer ends on exactly the byte where the varint must
    * terminate, but the final byte does not terminate.  The absolutely most
@@ -66,29 +66,31 @@ static void test_get_v_uint64_t()
    * then receive a UPB_ERROR_UNTERMINATED_VARINT error; clients who have no
    * more data to supply will (rightly) conclude that their protobuf is corrupt.
    */
-  status = upb_get_v_uint64_t(twelvebyte_buf, twelvebyte + 10, &twelvebyte_val, &twelvebyte_buf);
-  ASSERT(status == UPB_ERROR_UNTERMINATED_VARINT ||
-         status == UPB_STATUS_NEED_MORE_DATA);
+  upb_reset(&status);
+  upb_get_v_uint64_t(twelvebyte, twelvebyte + 10, &twelvebyte_val, &status);
+  ASSERT(status.code == UPB_ERROR_UNTERMINATED_VARINT ||
+         status.code == UPB_STATUS_NEED_MORE_DATA);
 
-  status = upb_get_v_uint64_t(twelvebyte_buf, twelvebyte + 9, &twelvebyte_val, &twelvebyte_buf);
-  ASSERT(status == UPB_STATUS_NEED_MORE_DATA);
+  upb_reset(&status);
+  upb_get_v_uint64_t(twelvebyte, twelvebyte + 9, &twelvebyte_val, &status);
+  ASSERT(status.code == UPB_STATUS_NEED_MORE_DATA);
 }
 
 static void test_get_v_uint32_t()
 {
 #define TEST(name, bytes, val) {\
-    upb_status_t status; \
+    struct upb_status status = UPB_STATUS_INIT; \
     uint8_t name[] = bytes; \
     uint8_t *name ## _buf = name; \
     uint32_t name ## _val = 0; \
-    status = upb_get_v_uint32_t(name, name + sizeof(name), &name ## _val, &name ## _buf); \
-    ASSERT(status == UPB_STATUS_OK); \
+    name ## _buf = upb_get_v_uint32_t(name, name + sizeof(name), &name ## _val, &status); \
+    ASSERT(upb_ok(&status)); \
     ASSERT(name ## _val == val); \
     ASSERT(name ## _buf == name + sizeof(name) - 1);  /* - 1 for NULL */ \
     /* Test NEED_MORE_DATA. */ \
     if(sizeof(name) > 2) { \
-      status = upb_get_v_uint32_t(name, name + sizeof(name) - 2, &name ## _val, &name ## _buf); \
-      ASSERT(status == UPB_STATUS_NEED_MORE_DATA); \
+      name ## _buf = upb_get_v_uint32_t(name, name + sizeof(name) - 2, &name ## _val, &status); \
+      ASSERT(status.code == UPB_STATUS_NEED_MORE_DATA); \
     } \
   }
 
@@ -107,18 +109,18 @@ static void test_get_v_uint32_t()
 #undef TEST
 
   uint8_t twelvebyte[] = {0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01, 0x01};
-  uint8_t *twelvebyte_buf = twelvebyte;
   uint32_t twelvebyte_val = 0;
-  upb_status_t status;
+  struct upb_status status = UPB_STATUS_INIT;
   /* A varint that terminates before hitting the end of the provided buffer,
    * but in too many bytes (11 instead of 10). */
-  status = upb_get_v_uint32_t(twelvebyte_buf, twelvebyte + 12, &twelvebyte_val, &twelvebyte_buf);
-  ASSERT(status == UPB_ERROR_UNTERMINATED_VARINT);
+  upb_get_v_uint32_t(twelvebyte, twelvebyte + 12, &twelvebyte_val, &status);
+  ASSERT(status.code == UPB_ERROR_UNTERMINATED_VARINT);
 
   /* A varint that terminates simultaneously with the end of the provided
    * buffer, but in too many bytes (11 instead of 10). */
-  status = upb_get_v_uint32_t(twelvebyte_buf, twelvebyte + 11, &twelvebyte_val, &twelvebyte_buf);
-  ASSERT(status == UPB_ERROR_UNTERMINATED_VARINT);
+  upb_reset(&status);
+  upb_get_v_uint32_t(twelvebyte, twelvebyte + 11, &twelvebyte_val, &status);
+  ASSERT(status.code == UPB_ERROR_UNTERMINATED_VARINT);
 
   /* A varint whose buffer ends on exactly the byte where the varint must
    * terminate, but the final byte does not terminate.  The absolutely most
@@ -129,27 +131,29 @@ static void test_get_v_uint32_t()
    * then receive a UPB_ERROR_UNTERMINATED_VARINT error; clients who have no
    * more data to supply will (rightly) conclude that their protobuf is corrupt.
    */
-  status = upb_get_v_uint32_t(twelvebyte_buf, twelvebyte + 10, &twelvebyte_val, &twelvebyte_buf);
-  ASSERT(status == UPB_ERROR_UNTERMINATED_VARINT ||
-         status == UPB_STATUS_NEED_MORE_DATA);
+  upb_reset(&status);
+  upb_get_v_uint32_t(twelvebyte, twelvebyte + 10, &twelvebyte_val, &status);
+  ASSERT(status.code == UPB_ERROR_UNTERMINATED_VARINT ||
+         status.code == UPB_STATUS_NEED_MORE_DATA);
 
-  status = upb_get_v_uint32_t(twelvebyte_buf, twelvebyte + 9, &twelvebyte_val, &twelvebyte_buf);
-  ASSERT(status == UPB_STATUS_NEED_MORE_DATA);
+  upb_reset(&status);
+  upb_get_v_uint32_t(twelvebyte, twelvebyte + 9, &twelvebyte_val, &status);
+  ASSERT(status.code == UPB_STATUS_NEED_MORE_DATA);
 }
 
 static void test_skip_v_uint64_t()
 {
 #define TEST(name, bytes) {\
-    upb_status_t status; \
+    struct upb_status status = UPB_STATUS_INIT; \
     uint8_t name[] = bytes; \
     uint8_t *name ## _buf = name; \
-    status = upb_skip_v_uint64_t(name ## _buf, name + sizeof(name), &name ## _buf); \
-    ASSERT(status == UPB_STATUS_OK); \
+    name ## _buf = upb_skip_v_uint64_t(name ## _buf, name + sizeof(name), &status); \
+    ASSERT(upb_ok(&status)); \
     ASSERT(name ## _buf == name + sizeof(name) - 1);  /* - 1 for NULL */ \
     /* Test NEED_MORE_DATA. */ \
     if(sizeof(name) > 2) { \
-      status = upb_skip_v_uint64_t(name, name + sizeof(name) - 2, &name ## _buf); \
-      ASSERT(status == UPB_STATUS_NEED_MORE_DATA); \
+      name ## _buf = upb_skip_v_uint64_t(name, name + sizeof(name) - 2, &status); \
+      ASSERT(status.code == UPB_STATUS_NEED_MORE_DATA); \
     } \
   }
 
@@ -167,17 +171,17 @@ static void test_skip_v_uint64_t()
 #undef TEST
 
   uint8_t twelvebyte[] = {0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01, 0x01};
-  uint8_t *twelvebyte_buf = twelvebyte;
-  upb_status_t status;
+  struct upb_status status = UPB_STATUS_INIT;
   /* A varint that terminates before hitting the end of the provided buffer,
    * but in too many bytes (11 instead of 10). */
-  status = upb_skip_v_uint64_t(twelvebyte_buf, twelvebyte + 12, &twelvebyte_buf);
-  ASSERT(status == UPB_ERROR_UNTERMINATED_VARINT);
+  upb_skip_v_uint64_t(twelvebyte, twelvebyte + 12, &status);
+  ASSERT(status.code == UPB_ERROR_UNTERMINATED_VARINT);
 
   /* A varint that terminates simultaneously with the end of the provided
    * buffer, but in too many bytes (11 instead of 10). */
-  status = upb_skip_v_uint64_t(twelvebyte_buf, twelvebyte + 11, &twelvebyte_buf);
-  ASSERT(status == UPB_ERROR_UNTERMINATED_VARINT);
+  upb_reset(&status);
+  upb_skip_v_uint64_t(twelvebyte, twelvebyte + 11, &status);
+  ASSERT(status.code == UPB_ERROR_UNTERMINATED_VARINT);
 
   /* A varint whose buffer ends on exactly the byte where the varint must
    * terminate, but the final byte does not terminate.  The absolutely most
@@ -188,23 +192,25 @@ static void test_skip_v_uint64_t()
    * then receive a UPB_ERROR_UNTERMINATED_VARINT error; clients who have no
    * more data to supply will (rightly) conclude that their protobuf is corrupt.
    */
-  status = upb_skip_v_uint64_t(twelvebyte_buf, twelvebyte + 10, &twelvebyte_buf);
-  ASSERT(status == UPB_ERROR_UNTERMINATED_VARINT ||
-         status == UPB_STATUS_NEED_MORE_DATA);
+  upb_reset(&status);
+  upb_skip_v_uint64_t(twelvebyte, twelvebyte + 10, &status);
+  ASSERT(status.code == UPB_ERROR_UNTERMINATED_VARINT ||
+         status.code == UPB_STATUS_NEED_MORE_DATA);
 
-  status = upb_skip_v_uint64_t(twelvebyte_buf, twelvebyte + 9, &twelvebyte_buf);
-  ASSERT(status == UPB_STATUS_NEED_MORE_DATA);
+  upb_reset(&status);
+  upb_skip_v_uint64_t(twelvebyte, twelvebyte + 9, &status);
+  ASSERT(status.code == UPB_STATUS_NEED_MORE_DATA);
 }
 
 static void test_get_f_uint32_t()
 {
 #define TEST(name, bytes, val) {\
-    upb_status_t status; \
+    struct upb_status status = UPB_STATUS_INIT; \
     uint8_t name[] = bytes; \
     uint8_t *name ## _buf = name; \
     uint32_t name ## _val = 0; \
-    status = upb_get_f_uint32_t(name ## _buf, name + sizeof(name), &name ## _val, &name ## _buf); \
-    ASSERT(status == UPB_STATUS_OK); \
+    name ## _buf = upb_get_f_uint32_t(name ## _buf, name + sizeof(name), &name ## _val, &status); \
+    ASSERT(upb_ok(&status)); \
     ASSERT(name ## _val == val); \
     ASSERT(name ## _buf == name + sizeof(name) - 1);  /* - 1 for NULL */ \
   }
@@ -213,10 +219,10 @@ static void test_get_f_uint32_t()
   TEST(one,   "\x01\x00\x00\x00",                                0x1UL);
 
   uint8_t threeb[] = {0x00, 0x00, 0x00};
-  uint8_t *threeb_buf = threeb;
   uint32_t threeb_val;
-  upb_status_t status = upb_get_f_uint32_t(threeb, threeb + sizeof(threeb), &threeb_val, &threeb_buf);
-  ASSERT(status == UPB_STATUS_NEED_MORE_DATA);
+  struct upb_status status = UPB_STATUS_INIT;
+  upb_get_f_uint32_t(threeb, threeb + sizeof(threeb), &threeb_val, &status);
+  ASSERT(status.code == UPB_STATUS_NEED_MORE_DATA);
 
 #undef TEST
 }
@@ -243,7 +249,9 @@ static void test_cbparser()
   ASSERT(p);
   upb_cbparser_reset(p, NULL, tag_cb, NULL, NULL, NULL, NULL);
   size_t read;
-  ASSERT(upb_cbparser_parse(p, NULL, 0, &read) == UPB_STATUS_OK);
+  struct upb_status status = UPB_STATUS_INIT;
+  read = upb_cbparser_parse(p, NULL, 0, &status);
+  ASSERT(upb_ok(&status));
   ASSERT(read == 0);
 }
 

tools/upbc.c

@@ -10,6 +10,7 @@
 
 #include <ctype.h>
 #include <inttypes.h>
+#include <stdarg.h>
 #include "descriptor.h"
 #include "upb_context.h"
 #include "upb_enum.h"
@@ -632,9 +633,13 @@ void usage_err(char *err)
   exit(1);
 }
 
-void error(char *err)
+void error(char *err, ...)
 {
-  fprintf(stderr, "upbc: %s\n\n", err);
+  va_list args;
+  va_start(args, err);
+  fprintf(stderr, "upbc: ");
+  vfprintf(stderr, err, args);
+  va_end(args);
   exit(1);
 }
 
@@ -680,12 +685,15 @@ int main(int argc, char *argv[])
   /* Parse input file. */
   struct upb_context *c = upb_context_new();
   struct upb_msg *fds_msg = upb_msg_new(c->fds_msg);
-  if(upb_msg_parsestr(fds_msg, descriptor->ptr, descriptor->byte_len) != UPB_STATUS_OK)
-    error("Failed to parse input file descriptor.");
+  struct upb_status status = UPB_STATUS_INIT;
+  upb_msg_parsestr(fds_msg, descriptor->ptr, descriptor->byte_len, &status);
+  if(!upb_ok(&status))
+    error("Failed to parse input file descriptor: %s", status.msg);
   //upb_msg_print(fds_msg, false, stderr);
   google_protobuf_FileDescriptorSet *fds = (void*)fds_msg;
-  if(!upb_context_addfds(c, fds))
-    error("Failed to resolve symbols in descriptor.\n");
+  upb_context_addfds(c, fds, &status);
+  if(!upb_ok(&status))
+    error("Failed to resolve symbols in descriptor: %s", status.msg);
 
   /* We need to sort the fields of all the descriptors.  They will already be
    * sorted in the upb_msgs that we base our header file output on, so we must