diff options
author | Chris Fallin <cfallin@google.com> | 2015-01-26 13:52:51 -0800 |
---|---|---|
committer | Chris Fallin <cfallin@google.com> | 2015-01-26 13:52:51 -0800 |
commit | 07b8b0f28d1ad88c7c1c64e8707dab8537313c26 (patch) | |
tree | 8bb849a0e2d73c225428b21ba48db5c9be4caea8 /ruby/ext/google/protobuf_c/encode_decode.c | |
parent | 9de35e742167b1dd2941cefab6e99239207f8e2c (diff) | |
download | protobuf-07b8b0f28d1ad88c7c1c64e8707dab8537313c26.tar.gz protobuf-07b8b0f28d1ad88c7c1c64e8707dab8537313c26.tar.bz2 protobuf-07b8b0f28d1ad88c7c1c64e8707dab8537313c26.zip |
Addressed code-review comments.
Diffstat (limited to 'ruby/ext/google/protobuf_c/encode_decode.c')
-rw-r--r-- | ruby/ext/google/protobuf_c/encode_decode.c | 13 |
1 files changed, 8 insertions, 5 deletions
diff --git a/ruby/ext/google/protobuf_c/encode_decode.c b/ruby/ext/google/protobuf_c/encode_decode.c index 3a441be3..b9ecd456 100644 --- a/ruby/ext/google/protobuf_c/encode_decode.c +++ b/ruby/ext/google/protobuf_c/encode_decode.c @@ -349,8 +349,6 @@ static void *oneofsubmsg_handler(void *closure, MessageHeader* msg = closure; const oneof_handlerdata_t *oneofdata = hd; uint32_t oldcase = DEREF(msg, oneofdata->case_ofs, uint32_t); - DEREF(msg, oneofdata->case_ofs, uint32_t) = - oneofdata->oneof_case_num; VALUE subdesc = get_def_obj((void*)oneofdata->md); @@ -361,6 +359,11 @@ static void *oneofsubmsg_handler(void *closure, DEREF(msg, oneofdata->ofs, VALUE) = rb_class_new_instance(0, NULL, subklass); } + // Set the oneof case *after* allocating the new class instance -- see comment + // in layout_set() as to why. There are subtle interactions with possible GC + // points and oneof field type transitions. + DEREF(msg, oneofdata->case_ofs, uint32_t) = + oneofdata->oneof_case_num; VALUE submsg_rb = DEREF(msg, oneofdata->ofs, VALUE); MessageHeader* submsg; @@ -965,11 +968,11 @@ static void putmsg(VALUE msg_rb, const Descriptor* desc, uint32_t offset = desc->layout->fields[upb_fielddef_index(f)].offset + sizeof(MessageHeader); - uint32_t oneof_case_offset = - desc->layout->fields[upb_fielddef_index(f)].case_offset + - sizeof(MessageHeader); if (upb_fielddef_containingoneof(f)) { + uint32_t oneof_case_offset = + desc->layout->fields[upb_fielddef_index(f)].case_offset + + sizeof(MessageHeader); // For a oneof, check that this field is actually present -- skip all the // below if not. if (DEREF(msg, oneof_case_offset, uint32_t) != |