Fixed some bad bugs in upb_env.

Also added a unit test for upb_encoder that
demonstrates the bugs and the fix.

Makefile

@@ -288,6 +288,7 @@ C_TESTS = \
 
 CC_TESTS = \
   tests/pb/test_decoder \
+  tests/pb/test_encoder \
   tests/json/test_json \
   tests/test_cpp \
   tests/test_table \
@@ -316,12 +317,11 @@ tests/pb/test_varint: LIBS = lib/libupb.pb.a lib/libupb.a $(EXTRA_LIBS)
 tests/test_def: LIBS = $(LOAD_DESCRIPTOR_LIBS) lib/libupb.a $(EXTRA_LIBS)
 tests/test_handlers: LIBS = lib/libupb.descriptor.a lib/libupb.a $(EXTRA_LIBS)
 tests/pb/test_decoder: LIBS = lib/libupb.pb.a lib/libupb.a $(EXTRA_LIBS)
+tests/pb/test_encoder: LIBS = lib/libupb.pb.a lib/libupb.descriptor.a lib/libupb.a $(EXTRA_LIBS)
 tests/test_cpp: LIBS = $(LOAD_DESCRIPTOR_LIBS) lib/libupb.a $(EXTRA_LIBS)
 tests/test_table: LIBS = lib/libupb.a $(EXTRA_LIBS)
 tests/json/test_json: LIBS = lib/libupb.a lib/libupb.json.a $(EXTRA_LIBS)
 
-tests/test_def: tests/test.proto.pb
-
 tests/test.proto.pb: tests/test.proto
 	@# TODO: add .proto file parser to upb so this isn't necessary.
 	protoc tests/test.proto -otests/test.proto.pb

tests/pb/test_encoder.cc

@@ -0,0 +1,52 @@
+
+#include "tests/upb_test.h"
+#include "upb/bindings/stdc++/string.h"
+#include "upb/descriptor/descriptor.upb.h"
+#include "upb/pb/decoder.h"
+#include "upb/pb/encoder.h"
+#include "upb/pb/glue.h"
+
+std::string read_string(const char *filename) {
+  size_t len;
+  char *str = upb_readfile(filename, &len);
+  ASSERT(str);
+  if (!str) { return std::string(); }
+  std::string ret = std::string(str, len);
+  free(str);
+  return ret;
+}
+
+void test_pb_roundtrip() {
+  upb::reffed_ptr<const upb::MessageDef> md(
+      upbdefs::google::protobuf::FileDescriptorSet::MessageDef());
+  upb::reffed_ptr<const upb::Handlers> encoder_handlers(
+      upb::pb::Encoder::NewHandlers(md.get()));
+  upb::reffed_ptr<const upb::pb::DecoderMethod> method(
+      upb::pb::DecoderMethod::New(
+          upb::pb::DecoderMethodOptions(encoder_handlers.get())));
+
+  char buf[512];
+  upb::SeededAllocator alloc(buf, sizeof(buf));
+  upb::Environment env;
+  env.SetAllocator(&alloc);
+  std::string input = read_string("upb/descriptor/descriptor.pb");
+  std::string output;
+  upb::StringSink string_sink(&output);
+  upb::pb::Encoder* encoder =
+      upb::pb::Encoder::Create(&env, encoder_handlers.get(),
+                               string_sink.input());
+  upb::pb::Decoder* decoder =
+      upb::pb::Decoder::Create(&env, method.get(), encoder->input());
+  bool ok = upb::BufferSource::PutBuffer(input, decoder->input());
+  ASSERT(ok);
+  ASSERT(input == output);
+}
+
+extern "C" {
+int run_tests(int argc, char *argv[]) {
+  UPB_UNUSED(argc);
+  UPB_UNUSED(argv);
+  test_pb_roundtrip();
+  return 0;
+}
+}

upb/bindings/stdc++/string.h

@@ -23,6 +23,9 @@ class FillStringHandler {
   // TODO(haberman): add UpbBind/UpbMakeHandler support to BytesHandler so these
   // can be prettier callbacks.
   static void* StartString(void *c, const void *hd, size_t size) {
+    UPB_UNUSED(hd);
+    UPB_UNUSED(size);
+
     T* str = static_cast<T*>(c);
     str->clear();
     return c;
@@ -30,6 +33,9 @@ class FillStringHandler {
 
   static size_t StringBuf(void* c, const void* hd, const char* buf, size_t n,
                           const BufferHandle* h) {
+    UPB_UNUSED(hd);
+    UPB_UNUSED(h);
+
     T* str = static_cast<T*>(c);
     try {
       str->append(buf, n);

upb/env.c

@@ -68,6 +68,7 @@ static void *default_alloc(void *_ud, void *ptr, size_t oldsize, size_t size) {
        * updated to its new location. */
       if (block->next) block->next->prev = block;
       if (block->prev) block->prev->next = block;
+      if (ud->head == from) ud->head = block;
     }
   } else {
     /* Insert at head of linked list. */
@@ -96,7 +97,7 @@ static void default_alloc_cleanup(void *_ud) {
 
 static bool default_err(void *ud, const upb_status *status) {
   UPB_UNUSED(ud);
-  fprintf(stderr, "upb error: %s\n", upb_status_errmsg(status));
+  UPB_UNUSED(status);
   return false;
 }
 
@@ -217,7 +218,6 @@ static size_t align_up(size_t size) {
 UPB_FORCEINLINE static void *seeded_alloc(void *ud, void *ptr, size_t oldsize,
                                           size_t size) {
   upb_seededalloc *a = ud;
-  UPB_UNUSED(ptr);
 
   size = align_up(size);
 
@@ -235,7 +235,11 @@ UPB_FORCEINLINE static void *seeded_alloc(void *ud, void *ptr, size_t oldsize,
     /* Is `ptr` part of the user-provided initial block? Don't pass it to the
      * default allocator if so; otherwise, it may try to realloc() the block. */
     if (chptr >= a->mem_base && chptr < a->mem_limit) {
-      return a->alloc(a->alloc_ud, NULL, 0, size);
+      void *ret;
+      assert(chptr + oldsize <= a->mem_limit);
+      ret = a->alloc(a->alloc_ud, NULL, 0, size);
+      if (ret) memcpy(ret, ptr, oldsize);
+      return ret;
     } else {
       return a->alloc(a->alloc_ud, ptr, oldsize, size);
     }

upb/pb/encoder.h

@@ -56,7 +56,7 @@ class upb::pb::Encoder {
   static const size_t kSize = UPB_PB_ENCODER_SIZE;
 
  private:
-  UPB_DISALLOW_POD_OPS(Encoder, upb::pb::Encoder);
+  UPB_DISALLOW_POD_OPS(Encoder, upb::pb::Encoder)
 };
 
 #endif