From c3e9a57a6fff08640f3ad05e8df971d5ddb37d51 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Thu, 18 Jun 2015 12:20:33 -0700 Subject: [PATCH] Fixed some bad bugs in upb_env. Also added a unit test for upb_encoder that demonstrates the bugs and the fix. --- Makefile | 4 +-- tests/pb/test_encoder.cc | 52 +++++++++++++++++++++++++++++++++++ upb/bindings/stdc++/string.h | 6 ++++ upb/descriptor/descriptor.pb | Bin 0 -> 4137 bytes upb/env.c | 10 +++++-- upb/pb/encoder.h | 2 +- 6 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 tests/pb/test_encoder.cc create mode 100644 upb/descriptor/descriptor.pb diff --git a/Makefile b/Makefile index d67beb01cf..19e7c8e53a 100644 --- a/Makefile +++ b/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 diff --git a/tests/pb/test_encoder.cc b/tests/pb/test_encoder.cc new file mode 100644 index 0000000000..8a222f424a --- /dev/null +++ b/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 md( + upbdefs::google::protobuf::FileDescriptorSet::MessageDef()); + upb::reffed_ptr encoder_handlers( + upb::pb::Encoder::NewHandlers(md.get())); + upb::reffed_ptr 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; +} +} diff --git a/upb/bindings/stdc++/string.h b/upb/bindings/stdc++/string.h index 5137486501..20a08768ce 100644 --- a/upb/bindings/stdc++/string.h +++ b/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(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(c); try { str->append(buf, n); diff --git a/upb/descriptor/descriptor.pb b/upb/descriptor/descriptor.pb new file mode 100644 index 0000000000000000000000000000000000000000..bcf3473528523beed2313af0b53c22ab68b2971c GIT binary patch literal 4137 zcmcInOLN;)6vhuF*0JL#Hz{>9EiI;$w6qT6mJ-4$4<+@C6Pws!C>wDj-#DVOBqTX) zx`7|Sf(2`q%&=s`u;LH!Lx6w5x%XO%Y&kPwCiTj?_q-q9ITwDp1Gk6X(Q1zcrf+$n z>nHx!Jl_pnGT(RIzRl#<(eSvs3ky4z&9-Bn4hu;g^2c~s$=u1U6zg|u6HK*njy9KE z{VNBHQ_e{V@(vwvS(z1;%)_il%@?%KDp_$r0D8=0PLDa}StW-GRdNFsyy1~;nMUFY z@Jb=qVg4nfFOnBUaq$#N0~Q1**a*)&R>=#~@1#sWcT|{7J^__E!+~T{1XJ2y9VsRm z*I+ha{;6fM%8X$9Af>A#@6((rdlfP#uw17yD|p{e@$Oj6?oDz&f?3xKku9hcFnY!M z%_)2~Ye^dNDQJN^^i5`%ZjTw3bL>`B!FM~w*AawUNY_BJ>aRJNpVU5{$tR#3a~m;_ zyhtwT!5W4;_Fcy9lAIqy(P2TzdNNhKQi}EenO|8Hm&@-nn2#+reClAu%JNuE#cm={ zH9hk1fU}X0&a*aqLG<@z>5dY&{1D0$GUh-DLh6U2jTBOZ9JY*TA*K400!vfNWjtYJ zn87kWVm@|R23u(z6q_C~8(Sr_Qm%iPa%-B*T9dPoHOPxDQv~aS%dvK`FUbO^oYN4P zFoWzW(V!qA-D43bT!XnDJElWBG)}2KWY{H~uy!6uS^l@CD;|Zh`g<1gLg_pdyU*LY zvE4e{Z0cHulmW!SPP4Vq)w1z-W54_KiIzi{k_?CPAfF_7T-Ow|xg-TNzSOsoWoG;Y zX=ceBsDfs*)oN;mSZ1et(AeMA)L7>3LF=%sK`dcU?{qeHb*&gvY(DSm9jz3L;AWwU zxmbn1f4HZWF}QO=V`qn(g%tA>M`)=QOX)}{3u6aJL8@Dz$UGA$H#au*rqOD58?F6~ zCdMv4JJ3HrY#iv@7{A0>Ti@v7S+4p+1{S9Q=Xh1Ffg)2ZVdQ=)qg=!nya*<`j3mI2 z7xXkR>7vFB!hvi#-Y}FfD~I$V6y0GMA995%Sv?O6N04Iu zoeNBl4EQ$(MZO`7zR41lzM-eoh!z65A>4!|af%9=Z0)Z90<5g0HMR7YoY_L3= z8GKI(Du9=Tb>*1a0V5CLmZ0e~hxs&Q2K#o17Y22D#t98hDoaV~9VodtF9+6Z6z2M3 zK$h$2I!stz;~sE^Jsg+oN~f*s+t>oocVl0Qk+r622s?0@8LbOJMN8_x3)jxAO;Rl+ zsnBolEG>y7X|`Z#=vWSh&GQ)_l-w^|G5(f2MtbZWCaGmss_RfndPh`{{$b-Io^7?Z zb)(bxO2?i@2y8aGx^Zy0-)-#aTJC{b_-9u89)H>D9~mf*4>fWRu16bMz(V7t<%@t1 zs2pl>`f707aSfbyPLKLM-1MSt)}W?|a}smJt1_m)3b&J{|Cxo77=1D#0Uwm6n0CBC z%k}qCWuBz2ZSje-J|j^c5qyT1H`YXt%BAsbc)a|tA|BOMP=q5`)gxE5w@3lH|;#A;aL??T$|`&?8+ zE(XT(V<^NX;_k^u34*j(q_a3rP?{AO@uH8_nO!MVe?#;de-I%E6l@n8lmn`K7O$k@ zGtN?Lj_8+7C`A9UbMhVyPlOC*Gb?&2A9&P3)%d^_Cm2eb(H{5FbO*fi1Az+SWgPm{ Zvd6A?B-PFPu!zKyHf?GXi$m}%^B;7A=aK*b literal 0 HcmV?d00001 diff --git a/upb/env.c b/upb/env.c index cc7a951c1b..c42560af82 100644 --- a/upb/env.c +++ b/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); } diff --git a/upb/pb/encoder.h b/upb/pb/encoder.h index 167d33f3eb..cb5c79f04b 100644 --- a/upb/pb/encoder.h +++ b/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