From a1daeaba80570a673913f8184855219607a6f556 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Fri, 10 Jul 2015 11:56:06 -0700 Subject: Conform to C89/C90 variable declaration rules. While we are C99 in general, the Ruby build system for building C extensions enables several flags that throw warnings for C89/C90 variable ordering rules. To avoid spewing a million warnings (or trying to specifically override these warnings with command-line flags, which would be tricky and possibly fragile) we conform to Ruby's world of C89/C90. Change-Id: I0e03e62d95068dfdfde112df0fb16a248a2f32a0 --- ruby/ext/google/protobuf_c/defs.c | 156 ++++++++++++---------- ruby/ext/google/protobuf_c/encode_decode.c | 193 ++++++++++++++++------------ ruby/ext/google/protobuf_c/map.c | 24 ++-- ruby/ext/google/protobuf_c/message.c | 97 +++++++++----- ruby/ext/google/protobuf_c/protobuf.c | 3 +- ruby/ext/google/protobuf_c/repeated_field.c | 80 +++++++----- ruby/ext/google/protobuf_c/storage.c | 48 ++++--- 7 files changed, 354 insertions(+), 247 deletions(-) (limited to 'ruby/ext') diff --git a/ruby/ext/google/protobuf_c/defs.c b/ruby/ext/google/protobuf_c/defs.c index 8f9f33e2..a5ca1ba7 100644 --- a/ruby/ext/google/protobuf_c/defs.c +++ b/ruby/ext/google/protobuf_c/defs.c @@ -88,7 +88,7 @@ static upb_enumdef* check_enum_notfrozen(const upb_enumdef* def) { } \ #define DEFINE_SELF(type, var, rb_var) \ - type* var = ruby_to_ ## type(rb_var); + type* var = ruby_to_ ## type(rb_var) // Global singleton DescriptorPool. The user is free to create others, but this // is used by generated code. @@ -656,13 +656,13 @@ VALUE FieldDescriptor_label(VALUE _self) { VALUE FieldDescriptor_label_set(VALUE _self, VALUE label) { DEFINE_SELF(FieldDescriptor, self, _self); upb_fielddef* mut_def = check_field_notfrozen(self->fielddef); + upb_label_t upb_label = -1; + bool converted = false; + if (TYPE(label) != T_SYMBOL) { rb_raise(rb_eArgError, "Expected symbol for field label."); } - upb_label_t upb_label = -1; - bool converted = false; - #define CONVERT(upb, ruby) \ if (SYM2ID(label) == rb_intern( # ruby )) { \ upb_label = UPB_LABEL_ ## upb; \ @@ -740,10 +740,10 @@ VALUE FieldDescriptor_submsg_name(VALUE _self) { VALUE FieldDescriptor_submsg_name_set(VALUE _self, VALUE value) { DEFINE_SELF(FieldDescriptor, self, _self); upb_fielddef* mut_def = check_field_notfrozen(self->fielddef); + const char* str = get_str(value); if (!upb_fielddef_hassubdef(self->fielddef)) { rb_raise(rb_eTypeError, "FieldDescriptor does not have subdef."); } - const char* str = get_str(value); CHECK_UPB(upb_fielddef_setsubdefname(mut_def, str, &status), "Error setting submessage name"); return Qnil; @@ -760,10 +760,12 @@ VALUE FieldDescriptor_submsg_name_set(VALUE _self, VALUE value) { */ VALUE FieldDescriptor_subtype(VALUE _self) { DEFINE_SELF(FieldDescriptor, self, _self); + const upb_def* def; + if (!upb_fielddef_hassubdef(self->fielddef)) { return Qnil; } - const upb_def* def = upb_fielddef_subdef(self->fielddef); + def = upb_fielddef_subdef(self->fielddef); if (def == NULL) { return Qnil; } @@ -1187,14 +1189,15 @@ static VALUE msgdef_add_field(VALUE msgdef, */ VALUE MessageBuilderContext_optional(int argc, VALUE* argv, VALUE _self) { DEFINE_SELF(MessageBuilderContext, self, _self); + VALUE name, type, number, type_class; if (argc < 3) { rb_raise(rb_eArgError, "Expected at least 3 arguments."); } - VALUE name = argv[0]; - VALUE type = argv[1]; - VALUE number = argv[2]; - VALUE type_class = (argc > 3) ? argv[3] : Qnil; + name = argv[0]; + type = argv[1]; + number = argv[2]; + type_class = (argc > 3) ? argv[3] : Qnil; return msgdef_add_field(self->descriptor, "optional", name, type, number, type_class); @@ -1215,14 +1218,15 @@ VALUE MessageBuilderContext_optional(int argc, VALUE* argv, VALUE _self) { */ VALUE MessageBuilderContext_required(int argc, VALUE* argv, VALUE _self) { DEFINE_SELF(MessageBuilderContext, self, _self); + VALUE name, type, number, type_class; if (argc < 3) { rb_raise(rb_eArgError, "Expected at least 3 arguments."); } - VALUE name = argv[0]; - VALUE type = argv[1]; - VALUE number = argv[2]; - VALUE type_class = (argc > 3) ? argv[3] : Qnil; + name = argv[0]; + type = argv[1]; + number = argv[2]; + type_class = (argc > 3) ? argv[3] : Qnil; return msgdef_add_field(self->descriptor, "required", name, type, number, type_class); @@ -1239,14 +1243,15 @@ VALUE MessageBuilderContext_required(int argc, VALUE* argv, VALUE _self) { */ VALUE MessageBuilderContext_repeated(int argc, VALUE* argv, VALUE _self) { DEFINE_SELF(MessageBuilderContext, self, _self); + VALUE name, type, number, type_class; if (argc < 3) { rb_raise(rb_eArgError, "Expected at least 3 arguments."); } - VALUE name = argv[0]; - VALUE type = argv[1]; - VALUE number = argv[2]; - VALUE type_class = (argc > 3) ? argv[3] : Qnil; + name = argv[0]; + type = argv[1]; + number = argv[2]; + type_class = (argc > 3) ? argv[3] : Qnil; return msgdef_add_field(self->descriptor, "repeated", name, type, number, type_class); @@ -1266,15 +1271,17 @@ VALUE MessageBuilderContext_repeated(int argc, VALUE* argv, VALUE _self) { */ VALUE MessageBuilderContext_map(int argc, VALUE* argv, VALUE _self) { DEFINE_SELF(MessageBuilderContext, self, _self); + VALUE name, key_type, value_type, number, type_class; + VALUE mapentry_desc, mapentry_desc_name; if (argc < 4) { rb_raise(rb_eArgError, "Expected at least 4 arguments."); } - VALUE name = argv[0]; - VALUE key_type = argv[1]; - VALUE value_type = argv[2]; - VALUE number = argv[3]; - VALUE type_class = (argc > 4) ? argv[4] : Qnil; + name = argv[0]; + key_type = argv[1]; + value_type = argv[2]; + number = argv[3]; + type_class = (argc > 4) ? argv[4] : Qnil; // Validate the key type. We can't accept enums, messages, or floats/doubles // as map keys. (We exclude these explicitly, and the field-descriptor setter @@ -1290,55 +1297,67 @@ VALUE MessageBuilderContext_map(int argc, VALUE* argv, VALUE _self) { // Create a new message descriptor for the map entry message, and create a // repeated submessage field here with that type. - VALUE mapentry_desc = rb_class_new_instance(0, NULL, cDescriptor); - VALUE mapentry_desc_name = rb_funcall(self->descriptor, rb_intern("name"), 0); + mapentry_desc = rb_class_new_instance(0, NULL, cDescriptor); + mapentry_desc_name = rb_funcall(self->descriptor, rb_intern("name"), 0); mapentry_desc_name = rb_str_cat2(mapentry_desc_name, "_MapEntry_"); mapentry_desc_name = rb_str_cat2(mapentry_desc_name, rb_id2name(SYM2ID(name))); Descriptor_name_set(mapentry_desc, mapentry_desc_name); - // The 'mapentry' attribute has no Ruby setter because we do not want the user - // attempting to DIY the setup below; we want to ensure that the fields are - // correct. So we reach into the msgdef here to set the bit manually. - Descriptor* mapentry_desc_self = ruby_to_Descriptor(mapentry_desc); - upb_msgdef_setmapentry((upb_msgdef*)mapentry_desc_self->msgdef, true); - - // optional key = 1; - VALUE key_field = rb_class_new_instance(0, NULL, cFieldDescriptor); - FieldDescriptor_name_set(key_field, rb_str_new2("key")); - FieldDescriptor_label_set(key_field, ID2SYM(rb_intern("optional"))); - FieldDescriptor_number_set(key_field, INT2NUM(1)); - FieldDescriptor_type_set(key_field, key_type); - Descriptor_add_field(mapentry_desc, key_field); - - // optional value = 2; - VALUE value_field = rb_class_new_instance(0, NULL, cFieldDescriptor); - FieldDescriptor_name_set(value_field, rb_str_new2("value")); - FieldDescriptor_label_set(value_field, ID2SYM(rb_intern("optional"))); - FieldDescriptor_number_set(value_field, INT2NUM(2)); - FieldDescriptor_type_set(value_field, value_type); - if (type_class != Qnil) { - VALUE submsg_name = rb_str_new2("."); // prepend '.' to make name absolute. - submsg_name = rb_str_append(submsg_name, type_class); - FieldDescriptor_submsg_name_set(value_field, submsg_name); + { + // The 'mapentry' attribute has no Ruby setter because we do not want the + // user attempting to DIY the setup below; we want to ensure that the fields + // are correct. So we reach into the msgdef here to set the bit manually. + Descriptor* mapentry_desc_self = ruby_to_Descriptor(mapentry_desc); + upb_msgdef_setmapentry((upb_msgdef*)mapentry_desc_self->msgdef, true); } - Descriptor_add_field(mapentry_desc, value_field); - // Add the map-entry message type to the current builder, and use the type to - // create the map field itself. - Builder* builder_self = ruby_to_Builder(self->builder); - rb_ary_push(builder_self->pending_list, mapentry_desc); + { + // optional key = 1; + VALUE key_field = rb_class_new_instance(0, NULL, cFieldDescriptor); + FieldDescriptor_name_set(key_field, rb_str_new2("key")); + FieldDescriptor_label_set(key_field, ID2SYM(rb_intern("optional"))); + FieldDescriptor_number_set(key_field, INT2NUM(1)); + FieldDescriptor_type_set(key_field, key_type); + Descriptor_add_field(mapentry_desc, key_field); + } - VALUE map_field = rb_class_new_instance(0, NULL, cFieldDescriptor); - VALUE name_str = rb_str_new2(rb_id2name(SYM2ID(name))); - FieldDescriptor_name_set(map_field, name_str); - FieldDescriptor_number_set(map_field, number); - FieldDescriptor_label_set(map_field, ID2SYM(rb_intern("repeated"))); - FieldDescriptor_type_set(map_field, ID2SYM(rb_intern("message"))); - VALUE submsg_name = rb_str_new2("."); // prepend '.' to make name absolute. - submsg_name = rb_str_append(submsg_name, mapentry_desc_name); - FieldDescriptor_submsg_name_set(map_field, submsg_name); - Descriptor_add_field(self->descriptor, map_field); + { + // optional value = 2; + VALUE value_field = rb_class_new_instance(0, NULL, cFieldDescriptor); + FieldDescriptor_name_set(value_field, rb_str_new2("value")); + FieldDescriptor_label_set(value_field, ID2SYM(rb_intern("optional"))); + FieldDescriptor_number_set(value_field, INT2NUM(2)); + FieldDescriptor_type_set(value_field, value_type); + if (type_class != Qnil) { + VALUE submsg_name = rb_str_new2("."); // prepend '.' to make absolute. + submsg_name = rb_str_append(submsg_name, type_class); + FieldDescriptor_submsg_name_set(value_field, submsg_name); + } + Descriptor_add_field(mapentry_desc, value_field); + } + + { + // Add the map-entry message type to the current builder, and use the type + // to create the map field itself. + Builder* builder_self = ruby_to_Builder(self->builder); + rb_ary_push(builder_self->pending_list, mapentry_desc); + } + + { + VALUE map_field = rb_class_new_instance(0, NULL, cFieldDescriptor); + VALUE name_str = rb_str_new2(rb_id2name(SYM2ID(name))); + VALUE submsg_name; + + FieldDescriptor_name_set(map_field, name_str); + FieldDescriptor_number_set(map_field, number); + FieldDescriptor_label_set(map_field, ID2SYM(rb_intern("repeated"))); + FieldDescriptor_type_set(map_field, ID2SYM(rb_intern("message"))); + submsg_name = rb_str_new2("."); // prepend '.' to make name absolute. + submsg_name = rb_str_append(submsg_name, mapentry_desc_name); + FieldDescriptor_submsg_name_set(map_field, submsg_name); + Descriptor_add_field(self->descriptor, map_field); + } return Qnil; } @@ -1434,14 +1453,15 @@ VALUE OneofBuilderContext_initialize(VALUE _self, */ VALUE OneofBuilderContext_optional(int argc, VALUE* argv, VALUE _self) { DEFINE_SELF(OneofBuilderContext, self, _self); + VALUE name, type, number, type_class; if (argc < 3) { rb_raise(rb_eArgError, "Expected at least 3 arguments."); } - VALUE name = argv[0]; - VALUE type = argv[1]; - VALUE number = argv[2]; - VALUE type_class = (argc > 3) ? argv[3] : Qnil; + name = argv[0]; + type = argv[1]; + number = argv[2]; + type_class = (argc > 3) ? argv[3] : Qnil; return msgdef_add_field(self->descriptor, "optional", name, type, number, type_class); diff --git a/ruby/ext/google/protobuf_c/encode_decode.c b/ruby/ext/google/protobuf_c/encode_decode.c index fe249d45..68f3b75d 100644 --- a/ruby/ext/google/protobuf_c/encode_decode.c +++ b/ruby/ext/google/protobuf_c/encode_decode.c @@ -175,11 +175,11 @@ static void *appendsubmsg_handler(void *closure, const void *hd) { VALUE subdesc = get_def_obj((void*)submsgdata->md); VALUE subklass = Descriptor_msgclass(subdesc); + MessageHeader* submsg; VALUE submsg_rb = rb_class_new_instance(0, NULL, subklass); RepeatedField_push(ary, submsg_rb); - MessageHeader* submsg; TypedData_Get_Struct(submsg_rb, MessageHeader, &Message_type, submsg); return submsg; } @@ -191,14 +191,15 @@ static void *submsg_handler(void *closure, const void *hd) { VALUE subdesc = get_def_obj((void*)submsgdata->md); VALUE subklass = Descriptor_msgclass(subdesc); + VALUE submsg_rb; + MessageHeader* submsg; if (DEREF(msg, submsgdata->ofs, VALUE) == Qnil) { DEREF(msg, submsgdata->ofs, VALUE) = rb_class_new_instance(0, NULL, subklass); } - VALUE submsg_rb = DEREF(msg, submsgdata->ofs, VALUE); - MessageHeader* submsg; + submsg_rb = DEREF(msg, submsgdata->ofs, VALUE); TypedData_Get_Struct(submsg_rb, MessageHeader, &Message_type, submsg); return submsg; } @@ -254,12 +255,14 @@ static bool endmap_handler(void *closure, const void *hd, upb_status* s) { &frame->key_storage); VALUE value_field_typeclass = Qnil; + VALUE value; + if (mapdata->value_field_type == UPB_TYPE_MESSAGE || mapdata->value_field_type == UPB_TYPE_ENUM) { value_field_typeclass = get_def_obj(mapdata->value_field_subdef); } - VALUE value = native_slot_get( + value = native_slot_get( mapdata->value_field_type, value_field_typeclass, &frame->value_storage); @@ -280,15 +283,14 @@ static map_handlerdata_t* new_map_handlerdata( size_t ofs, const upb_msgdef* mapentry_def, Descriptor* desc) { - + const upb_fielddef* key_field; + const upb_fielddef* value_field; map_handlerdata_t* hd = ALLOC(map_handlerdata_t); hd->ofs = ofs; - const upb_fielddef* key_field = upb_msgdef_itof(mapentry_def, - MAP_KEY_FIELD); + key_field = upb_msgdef_itof(mapentry_def, MAP_KEY_FIELD); assert(key_field != NULL); hd->key_field_type = upb_fielddef_type(key_field); - const upb_fielddef* value_field = upb_msgdef_itof(mapentry_def, - MAP_VALUE_FIELD); + value_field = upb_msgdef_itof(mapentry_def, MAP_VALUE_FIELD); assert(value_field != NULL); hd->value_field_type = upb_fielddef_type(value_field); hd->value_field_subdef = upb_fielddef_subdef(value_field); @@ -354,6 +356,8 @@ static void *oneofsubmsg_handler(void *closure, VALUE subdesc = get_def_obj((void*)oneofdata->md); VALUE subklass = Descriptor_msgclass(subdesc); + VALUE submsg_rb; + MessageHeader* submsg; if (oldcase != oneofdata->oneof_case_num || DEREF(msg, oneofdata->ofs, VALUE) == Qnil) { @@ -369,8 +373,7 @@ static void *oneofsubmsg_handler(void *closure, DEREF(msg, oneofdata->case_ofs, uint32_t) = oneofdata->oneof_case_num; - VALUE submsg_rb = DEREF(msg, oneofdata->ofs, VALUE); - MessageHeader* submsg; + submsg_rb = DEREF(msg, oneofdata->ofs, VALUE); TypedData_Get_Struct(submsg_rb, MessageHeader, &Message_type, submsg); return submsg; } @@ -465,8 +468,9 @@ static void add_handlers_for_mapfield(upb_handlers* h, Descriptor* desc) { const upb_msgdef* map_msgdef = upb_fielddef_msgsubdef(fielddef); map_handlerdata_t* hd = new_map_handlerdata(offset, map_msgdef, desc); - upb_handlers_addcleanup(h, hd, free); upb_handlerattr attr = UPB_HANDLERATTR_INITIALIZER; + + upb_handlers_addcleanup(h, hd, free); upb_handlerattr_sethandlerdata(&attr, hd); upb_handlers_setstartsubmsg(h, fielddef, startmapentry_handler, &attr); upb_handlerattr_uninit(&attr); @@ -479,8 +483,9 @@ static void add_handlers_for_mapentry(const upb_msgdef* msgdef, const upb_fielddef* key_field = map_entry_key(msgdef); const upb_fielddef* value_field = map_entry_value(msgdef); map_handlerdata_t* hd = new_map_handlerdata(0, msgdef, desc); - upb_handlers_addcleanup(h, hd, free); upb_handlerattr attr = UPB_HANDLERATTR_INITIALIZER; + + upb_handlers_addcleanup(h, hd, free); upb_handlerattr_sethandlerdata(&attr, hd); upb_handlers_setendmsg(h, endmap_handler, &attr); @@ -542,6 +547,7 @@ static void add_handlers_for_oneof_field(upb_handlers *h, static void add_handlers_for_message(const void *closure, upb_handlers *h) { const upb_msgdef* msgdef = upb_handlers_msgdef(h); Descriptor* desc = ruby_to_Descriptor(get_def_obj((void*)msgdef)); + upb_msg_field_iter i; // If this is a mapentry message type, set up a special set of handlers and // bail out of the normal (user-defined) message type handling. @@ -558,7 +564,6 @@ static void add_handlers_for_message(const void *closure, upb_handlers *h) { desc->layout = create_layout(desc->msgdef); } - upb_msg_field_iter i; for (upb_msg_field_begin(&i, desc->msgdef); !upb_msg_field_done(&i); upb_msg_field_next(&i)) { @@ -610,8 +615,7 @@ const upb_pbdecodermethod *new_fillmsg_decodermethod(Descriptor* desc, upb_pbdecodermethodopts opts; upb_pbdecodermethodopts_init(&opts, handlers); - const upb_pbdecodermethod *ret = upb_pbdecodermethod_new(&opts, owner); - return ret; + return upb_pbdecodermethod_new(&opts, owner); } static const upb_pbdecodermethod *msgdef_decodermethod(Descriptor* desc) { @@ -677,28 +681,31 @@ VALUE Message_decode(VALUE klass, VALUE data) { VALUE descriptor = rb_ivar_get(klass, descriptor_instancevar_interned); Descriptor* desc = ruby_to_Descriptor(descriptor); VALUE msgklass = Descriptor_msgclass(descriptor); + VALUE msg_rb; + MessageHeader* msg; if (TYPE(data) != T_STRING) { rb_raise(rb_eArgError, "Expected string for binary protobuf data."); } - VALUE msg_rb = rb_class_new_instance(0, NULL, msgklass); - MessageHeader* msg; + msg_rb = rb_class_new_instance(0, NULL, msgklass); TypedData_Get_Struct(msg_rb, MessageHeader, &Message_type, msg); - const upb_pbdecodermethod* method = msgdef_decodermethod(desc); - const upb_handlers* h = upb_pbdecodermethod_desthandlers(method); - stackenv se; - stackenv_init(&se, "Error occurred during parsing: %s"); + { + const upb_pbdecodermethod* method = msgdef_decodermethod(desc); + const upb_handlers* h = upb_pbdecodermethod_desthandlers(method); + stackenv se; + upb_sink sink; + upb_pbdecoder* decoder; + stackenv_init(&se, "Error occurred during parsing: %s"); - upb_sink sink; - upb_sink_reset(&sink, h, msg); - upb_pbdecoder* decoder = - upb_pbdecoder_create(&se.env, method, &sink); - upb_bufsrc_putbuf(RSTRING_PTR(data), RSTRING_LEN(data), - upb_pbdecoder_input(decoder)); + upb_sink_reset(&sink, h, msg); + decoder = upb_pbdecoder_create(&se.env, method, &sink); + upb_bufsrc_putbuf(RSTRING_PTR(data), RSTRING_LEN(data), + upb_pbdecoder_input(decoder)); - stackenv_uninit(&se); + stackenv_uninit(&se); + } return msg_rb; } @@ -715,6 +722,8 @@ VALUE Message_decode_json(VALUE klass, VALUE data) { VALUE descriptor = rb_ivar_get(klass, descriptor_instancevar_interned); Descriptor* desc = ruby_to_Descriptor(descriptor); VALUE msgklass = Descriptor_msgclass(descriptor); + VALUE msg_rb; + MessageHeader* msg; if (TYPE(data) != T_STRING) { rb_raise(rb_eArgError, "Expected string for JSON data."); @@ -723,20 +732,22 @@ VALUE Message_decode_json(VALUE klass, VALUE data) { // convert, because string handlers pass data directly to message string // fields. - VALUE msg_rb = rb_class_new_instance(0, NULL, msgklass); - MessageHeader* msg; + msg_rb = rb_class_new_instance(0, NULL, msgklass); TypedData_Get_Struct(msg_rb, MessageHeader, &Message_type, msg); - stackenv se; - stackenv_init(&se, "Error occurred during parsing: %s"); + { + stackenv se; + upb_sink sink; + upb_json_parser* parser; + stackenv_init(&se, "Error occurred during parsing: %s"); - upb_sink sink; - upb_sink_reset(&sink, get_fill_handlers(desc), msg); - upb_json_parser* parser = upb_json_parser_create(&se.env, &sink); - upb_bufsrc_putbuf(RSTRING_PTR(data), RSTRING_LEN(data), - upb_json_parser_input(parser)); + upb_sink_reset(&sink, get_fill_handlers(desc), msg); + parser = upb_json_parser_create(&se.env, &sink); + upb_bufsrc_putbuf(RSTRING_PTR(data), RSTRING_LEN(data), + upb_json_parser_input(parser)); - stackenv_uninit(&se); + stackenv_uninit(&se); + } return msg_rb; } @@ -767,12 +778,12 @@ static void *stringsink_start(void *_sink, const void *hd, size_t size_hint) { static size_t stringsink_string(void *_sink, const void *hd, const char *ptr, size_t len, const upb_bufhandle *handle) { - UPB_UNUSED(hd); - UPB_UNUSED(handle); - stringsink *sink = _sink; size_t new_size = sink->size; + UPB_UNUSED(hd); + UPB_UNUSED(handle); + while (sink->len + len > new_size) { new_size *= 2; } @@ -826,10 +837,11 @@ static upb_selector_t getsel(const upb_fielddef *f, upb_handlertype_t type) { } static void putstr(VALUE str, const upb_fielddef *f, upb_sink *sink) { + upb_sink subsink; + if (str == Qnil) return; assert(BUILTIN_TYPE(str) == RUBY_T_STRING); - upb_sink subsink; // Ensure that the string has the correct encoding. We also check at field-set // time, but the user may have mutated the string object since then. @@ -844,11 +856,14 @@ static void putstr(VALUE str, const upb_fielddef *f, upb_sink *sink) { static void putsubmsg(VALUE submsg, const upb_fielddef *f, upb_sink *sink, int depth) { + upb_sink subsink; + VALUE descriptor; + Descriptor* subdesc; + if (submsg == Qnil) return; - upb_sink subsink; - VALUE descriptor = rb_ivar_get(submsg, descriptor_instancevar_interned); - Descriptor* subdesc = ruby_to_Descriptor(descriptor); + descriptor = rb_ivar_get(submsg, descriptor_instancevar_interned); + subdesc = ruby_to_Descriptor(descriptor); upb_sink_startsubmsg(sink, getsel(f, UPB_HANDLER_STARTSUBMSG), &subsink); putmsg(submsg, subdesc, &subsink, depth + 1); @@ -857,19 +872,20 @@ static void putsubmsg(VALUE submsg, const upb_fielddef *f, upb_sink *sink, static void putary(VALUE ary, const upb_fielddef *f, upb_sink *sink, int depth) { - if (ary == Qnil) return; - upb_sink subsink; + upb_fieldtype_t type = upb_fielddef_type(f); + upb_selector_t sel = 0; + int size; + + if (ary == Qnil) return; upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); - upb_fieldtype_t type = upb_fielddef_type(f); - upb_selector_t sel = 0; if (upb_fielddef_isprimitive(f)) { sel = getsel(f, upb_handlers_getprimitivehandlertype(f)); } - int size = NUM2INT(RepeatedField_length(ary)); + size = NUM2INT(RepeatedField_length(ary)); for (int i = 0; i < size; i++) { void* memory = RepeatedField_index_native(ary, i); switch (type) { @@ -952,21 +968,25 @@ static void put_ruby_value(VALUE value, static void putmap(VALUE map, const upb_fielddef *f, upb_sink *sink, int depth) { - if (map == Qnil) return; - Map* self = ruby_to_Map(map); - + Map* self; upb_sink subsink; + const upb_fielddef* key_field; + const upb_fielddef* value_field; + Map_iter it; + + if (map == Qnil) return; + self = ruby_to_Map(map); upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); assert(upb_fielddef_type(f) == UPB_TYPE_MESSAGE); - const upb_fielddef* key_field = map_field_key(f); - const upb_fielddef* value_field = map_field_value(f); + key_field = map_field_key(f); + value_field = map_field_value(f); - Map_iter it; for (Map_begin(map, &it); !Map_done(&it); Map_next(&it)) { VALUE key = Map_iter_key(&it); VALUE value = Map_iter_value(&it); + upb_status status; upb_sink entry_sink; upb_sink_startsubmsg(&subsink, getsel(f, UPB_HANDLER_STARTSUBMSG), @@ -977,7 +997,6 @@ static void putmap(VALUE map, const upb_fielddef *f, upb_sink *sink, put_ruby_value(value, value_field, self->value_type_class, depth + 1, &entry_sink); - upb_status status; upb_sink_endmsg(&entry_sink, &status); upb_sink_endsubmsg(&subsink, getsel(f, UPB_HANDLER_ENDSUBMSG)); } @@ -987,6 +1006,10 @@ static void putmap(VALUE map, const upb_fielddef *f, upb_sink *sink, static void putmsg(VALUE msg_rb, const Descriptor* desc, upb_sink *sink, int depth) { + MessageHeader* msg; + upb_msg_field_iter i; + upb_status status; + upb_sink_startmsg(sink); // Protect against cycles (possible because users may freely reassign message @@ -996,10 +1019,8 @@ static void putmsg(VALUE msg_rb, const Descriptor* desc, "Maximum recursion depth exceeded during encoding."); } - MessageHeader* msg; TypedData_Get_Struct(msg_rb, MessageHeader, &Message_type, msg); - upb_msg_field_iter i; for (upb_msg_field_begin(&i, desc->msgdef); !upb_msg_field_done(&i); upb_msg_field_next(&i)) { @@ -1071,7 +1092,6 @@ static void putmsg(VALUE msg_rb, const Descriptor* desc, } } - upb_status status; upb_sink_endmsg(sink, &status); } @@ -1106,22 +1126,26 @@ VALUE Message_encode(VALUE klass, VALUE msg_rb) { stringsink sink; stringsink_init(&sink); - const upb_handlers* serialize_handlers = - msgdef_pb_serialize_handlers(desc); + { + const upb_handlers* serialize_handlers = + msgdef_pb_serialize_handlers(desc); - stackenv se; - stackenv_init(&se, "Error occurred during encoding: %s"); - upb_pb_encoder* encoder = - upb_pb_encoder_create(&se.env, serialize_handlers, &sink.sink); + stackenv se; + upb_pb_encoder* encoder; + VALUE ret; - putmsg(msg_rb, desc, upb_pb_encoder_input(encoder), 0); + stackenv_init(&se, "Error occurred during encoding: %s"); + encoder = upb_pb_encoder_create(&se.env, serialize_handlers, &sink.sink); - VALUE ret = rb_str_new(sink.ptr, sink.len); + putmsg(msg_rb, desc, upb_pb_encoder_input(encoder), 0); - stackenv_uninit(&se); - stringsink_uninit(&sink); + ret = rb_str_new(sink.ptr, sink.len); - return ret; + stackenv_uninit(&se); + stringsink_uninit(&sink); + + return ret; + } } /* @@ -1137,21 +1161,24 @@ VALUE Message_encode_json(VALUE klass, VALUE msg_rb) { stringsink sink; stringsink_init(&sink); - const upb_handlers* serialize_handlers = - msgdef_json_serialize_handlers(desc); + { + const upb_handlers* serialize_handlers = + msgdef_json_serialize_handlers(desc); + upb_json_printer* printer; + stackenv se; + VALUE ret; - stackenv se; - stackenv_init(&se, "Error occurred during encoding: %s"); - upb_json_printer* printer = - upb_json_printer_create(&se.env, serialize_handlers, &sink.sink); + stackenv_init(&se, "Error occurred during encoding: %s"); + printer = upb_json_printer_create(&se.env, serialize_handlers, &sink.sink); - putmsg(msg_rb, desc, upb_json_printer_input(printer), 0); + putmsg(msg_rb, desc, upb_json_printer_input(printer), 0); - VALUE ret = rb_str_new(sink.ptr, sink.len); + ret = rb_str_new(sink.ptr, sink.len); - stackenv_uninit(&se); - stringsink_uninit(&sink); + stackenv_uninit(&se); + stringsink_uninit(&sink); - return ret; + return ret; + } } diff --git a/ruby/ext/google/protobuf_c/map.c b/ruby/ext/google/protobuf_c/map.c index 3436e09a..5043f395 100644 --- a/ruby/ext/google/protobuf_c/map.c +++ b/ruby/ext/google/protobuf_c/map.c @@ -167,9 +167,9 @@ void Map_free(void* _self) { VALUE Map_alloc(VALUE klass) { Map* self = ALLOC(Map); + VALUE ret = TypedData_Wrap_Struct(klass, &Map_type, self); memset(self, 0, sizeof(Map)); self->value_type_class = Qnil; - VALUE ret = TypedData_Wrap_Struct(klass, &Map_type, self); return ret; } @@ -215,6 +215,7 @@ static bool needs_typeclass(upb_fieldtype_t type) { */ VALUE Map_init(int argc, VALUE* argv, VALUE _self) { Map* self = ruby_to_Map(_self); + int init_value_arg; // We take either two args (:key_type, :value_type), three args (:key_type, // :value_type, "ValueMessageType"), or four args (the above plus an initial @@ -241,7 +242,7 @@ VALUE Map_init(int argc, VALUE* argv, VALUE _self) { rb_raise(rb_eArgError, "Invalid key type for map."); } - int init_value_arg = 2; + init_value_arg = 2; if (needs_typeclass(self->value_type) && argc > 2) { self->value_type_class = argv[2]; validate_type_class(self->value_type, self->value_type_class); @@ -356,9 +357,9 @@ VALUE Map_index(VALUE _self, VALUE key) { char keybuf[TABLE_KEY_BUF_LENGTH]; const char* keyval = NULL; size_t length = 0; + upb_value v; table_key(self, key, keybuf, &keyval, &length); - upb_value v; if (upb_strtable_lookup2(&self->table, keyval, length, &v)) { void* mem = value_memory(&v); return native_slot_get(self->value_type, self->value_type_class, mem); @@ -381,10 +382,11 @@ VALUE Map_index_set(VALUE _self, VALUE key, VALUE value) { char keybuf[TABLE_KEY_BUF_LENGTH]; const char* keyval = NULL; size_t length = 0; + upb_value v; + void* mem; table_key(self, key, keybuf, &keyval, &length); - upb_value v; - void* mem = value_memory(&v); + mem = value_memory(&v); native_slot_set(self->value_type, self->value_type_class, mem, value); // Replace any existing value by issuing a 'remove' operation first. @@ -432,9 +434,9 @@ VALUE Map_delete(VALUE _self, VALUE key) { char keybuf[TABLE_KEY_BUF_LENGTH]; const char* keyval = NULL; size_t length = 0; + upb_value v; table_key(self, key, keybuf, &keyval, &length); - upb_value v; if (upb_strtable_remove2(&self->table, keyval, length, &v)) { void* mem = value_memory(&v); return native_slot_get(self->value_type, self->value_type_class, mem); @@ -564,6 +566,8 @@ VALUE Map_deep_copy(VALUE _self) { */ VALUE Map_eq(VALUE _self, VALUE _other) { Map* self = ruby_to_Map(_self); + Map* other; + upb_strtable_iter it; // Allow comparisons to Ruby hashmaps by converting to a temporary Map // instance. Slow, but workable. @@ -573,7 +577,7 @@ VALUE Map_eq(VALUE _self, VALUE _other) { _other = other_map; } - Map* other = ruby_to_Map(_other); + other = ruby_to_Map(_other); if (self == other) { return Qtrue; @@ -589,7 +593,6 @@ VALUE Map_eq(VALUE _self, VALUE _other) { // For each member of self, check that an equal member exists at the same key // in other. - upb_strtable_iter it; for (upb_strtable_begin(&it, &self->table); !upb_strtable_done(&it); upb_strtable_next(&it)) { @@ -719,6 +722,7 @@ VALUE Map_merge_into_self(VALUE _self, VALUE hashmap) { Map* self = ruby_to_Map(_self); Map* other = ruby_to_Map(hashmap); + upb_strtable_iter it; if (self->key_type != other->key_type || self->value_type != other->value_type || @@ -726,19 +730,19 @@ VALUE Map_merge_into_self(VALUE _self, VALUE hashmap) { rb_raise(rb_eArgError, "Attempt to merge Map with mismatching types"); } - upb_strtable_iter it; for (upb_strtable_begin(&it, &other->table); !upb_strtable_done(&it); upb_strtable_next(&it)) { // Replace any existing value by issuing a 'remove' operation first. + upb_value v; upb_value oldv; upb_strtable_remove2(&self->table, upb_strtable_iter_key(&it), upb_strtable_iter_keylength(&it), &oldv); - upb_value v = upb_strtable_iter_value(&it); + v = upb_strtable_iter_value(&it); upb_strtable_insert2(&self->table, upb_strtable_iter_key(&it), upb_strtable_iter_keylength(&it), diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index 4be1c8c8..ebe2f1ab 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -57,11 +57,13 @@ VALUE Message_alloc(VALUE klass) { Descriptor* desc = ruby_to_Descriptor(descriptor); MessageHeader* msg = (MessageHeader*)ALLOC_N( uint8_t, sizeof(MessageHeader) + desc->layout->size); + VALUE ret; + memset(Message_data(msg), 0, desc->layout->size); // We wrap first so that everything in the message object is GC-rooted in case // a collection happens during object creation in layout_init(). - VALUE ret = TypedData_Wrap_Struct(klass, &Message_type, msg); + ret = TypedData_Wrap_Struct(klass, &Message_type, msg); msg->descriptor = desc; rb_ivar_set(ret, descriptor_instancevar_interned, descriptor); @@ -71,29 +73,34 @@ VALUE Message_alloc(VALUE klass) { } static VALUE which_oneof_field(MessageHeader* self, const upb_oneofdef* o) { + upb_oneof_iter it; + size_t case_ofs; + uint32_t oneof_case; + const upb_fielddef* first_field; + const upb_fielddef* f; + // If no fields in the oneof, always nil. if (upb_oneofdef_numfields(o) == 0) { return Qnil; } // Grab the first field in the oneof so we can get its layout info to find the // oneof_case field. - upb_oneof_iter it; upb_oneof_begin(&it, o); assert(!upb_oneof_done(&it)); - const upb_fielddef* first_field = upb_oneof_iter_field(&it); + first_field = upb_oneof_iter_field(&it); assert(upb_fielddef_containingoneof(first_field) != NULL); - size_t case_ofs = + case_ofs = self->descriptor->layout-> fields[upb_fielddef_index(first_field)].case_offset; - uint32_t oneof_case = *((uint32_t*)((char*)Message_data(self) + case_ofs)); + oneof_case = *((uint32_t*)((char*)Message_data(self) + case_ofs)); if (oneof_case == ONEOF_CASE_NONE) { return Qnil; } // oneof_case is a field index, so find that field. - const upb_fielddef* f = upb_oneofdef_itof(o, oneof_case); + f = upb_oneofdef_itof(o, oneof_case); assert(f != NULL); return ID2SYM(rb_intern(upb_fielddef_name(f))); @@ -118,18 +125,25 @@ static VALUE which_oneof_field(MessageHeader* self, const upb_oneofdef* o) { */ VALUE Message_method_missing(int argc, VALUE* argv, VALUE _self) { MessageHeader* self; + VALUE method_name, method_str; + char* name; + size_t name_len; + bool setter; + const upb_oneofdef* o; + const upb_fielddef* f; + TypedData_Get_Struct(_self, MessageHeader, &Message_type, self); if (argc < 1) { rb_raise(rb_eArgError, "Expected method name as first argument."); } - VALUE method_name = argv[0]; + method_name = argv[0]; if (!SYMBOL_P(method_name)) { rb_raise(rb_eArgError, "Expected symbol as method name."); } - VALUE method_str = rb_id2str(SYM2ID(method_name)); - char* name = RSTRING_PTR(method_str); - size_t name_len = RSTRING_LEN(method_str); - bool setter = false; + method_str = rb_id2str(SYM2ID(method_name)); + name = RSTRING_PTR(method_str); + name_len = RSTRING_LEN(method_str); + setter = false; // Setters have names that end in '='. if (name[name_len - 1] == '=') { @@ -138,7 +152,7 @@ VALUE Message_method_missing(int argc, VALUE* argv, VALUE _self) { } // Check for a oneof name first. - const upb_oneofdef* o = upb_msgdef_ntoo(self->descriptor->msgdef, + o = upb_msgdef_ntoo(self->descriptor->msgdef, name, name_len); if (o != NULL) { if (setter) { @@ -148,7 +162,7 @@ VALUE Message_method_missing(int argc, VALUE* argv, VALUE _self) { } // Otherwise, check for a field with that name. - const upb_fielddef* f = upb_msgdef_ntof(self->descriptor->msgdef, + f = upb_msgdef_ntof(self->descriptor->msgdef, name, name_len); if (f == NULL) { @@ -168,6 +182,9 @@ VALUE Message_method_missing(int argc, VALUE* argv, VALUE _self) { int Message_initialize_kwarg(VALUE key, VALUE val, VALUE _self) { MessageHeader* self; + VALUE method_str; + char* name; + const upb_fielddef* f; TypedData_Get_Struct(_self, MessageHeader, &Message_type, self); if (!SYMBOL_P(key)) { @@ -175,27 +192,31 @@ int Message_initialize_kwarg(VALUE key, VALUE val, VALUE _self) { "Expected symbols as hash keys in initialization map."); } - VALUE method_str = rb_id2str(SYM2ID(key)); - char* name = RSTRING_PTR(method_str); - const upb_fielddef* f = upb_msgdef_ntofz(self->descriptor->msgdef, name); + method_str = rb_id2str(SYM2ID(key)); + name = RSTRING_PTR(method_str); + f = upb_msgdef_ntofz(self->descriptor->msgdef, name); if (f == NULL) { rb_raise(rb_eArgError, "Unknown field name in initialization map entry."); } if (is_map_field(f)) { + VALUE map; + if (TYPE(val) != T_HASH) { rb_raise(rb_eArgError, "Expected Hash object as initializer value for map field."); } - VALUE map = layout_get(self->descriptor->layout, Message_data(self), f); + map = layout_get(self->descriptor->layout, Message_data(self), f); Map_merge_into_self(map, val); } else if (upb_fielddef_label(f) == UPB_LABEL_REPEATED) { + VALUE ary; + if (TYPE(val) != T_ARRAY) { rb_raise(rb_eArgError, "Expected array as initializer value for repeated field."); } - VALUE ary = layout_get(self->descriptor->layout, Message_data(self), f); + ary = layout_get(self->descriptor->layout, Message_data(self), f); for (int i = 0; i < RARRAY_LEN(val); i++) { RepeatedField_push(ary, rb_ary_entry(val, i)); } @@ -218,13 +239,15 @@ int Message_initialize_kwarg(VALUE key, VALUE val, VALUE _self) { * Message class are provided on each concrete message class. */ VALUE Message_initialize(int argc, VALUE* argv, VALUE _self) { + VALUE hash_args; + if (argc == 0) { return Qnil; } if (argc != 1) { rb_raise(rb_eArgError, "Expected 0 or 1 arguments."); } - VALUE hash_args = argv[0]; + hash_args = argv[0]; if (TYPE(hash_args) != T_HASH) { rb_raise(rb_eArgError, "Expected hash arguments."); } @@ -241,10 +264,11 @@ VALUE Message_initialize(int argc, VALUE* argv, VALUE _self) { */ VALUE Message_dup(VALUE _self) { MessageHeader* self; + VALUE new_msg; + MessageHeader* new_msg_self; TypedData_Get_Struct(_self, MessageHeader, &Message_type, self); - VALUE new_msg = rb_class_new_instance(0, NULL, CLASS_OF(_self)); - MessageHeader* new_msg_self; + new_msg = rb_class_new_instance(0, NULL, CLASS_OF(_self)); TypedData_Get_Struct(new_msg, MessageHeader, &Message_type, new_msg_self); layout_dup(self->descriptor->layout, @@ -257,10 +281,11 @@ VALUE Message_dup(VALUE _self) { // Internal only; used by Google::Protobuf.deep_copy. VALUE Message_deep_copy(VALUE _self) { MessageHeader* self; + MessageHeader* new_msg_self; + VALUE new_msg; TypedData_Get_Struct(_self, MessageHeader, &Message_type, self); - VALUE new_msg = rb_class_new_instance(0, NULL, CLASS_OF(_self)); - MessageHeader* new_msg_self; + new_msg = rb_class_new_instance(0, NULL, CLASS_OF(_self)); TypedData_Get_Struct(new_msg, MessageHeader, &Message_type, new_msg_self); layout_deep_copy(self->descriptor->layout, @@ -281,9 +306,8 @@ VALUE Message_deep_copy(VALUE _self) { */ VALUE Message_eq(VALUE _self, VALUE _other) { MessageHeader* self; - TypedData_Get_Struct(_self, MessageHeader, &Message_type, self); - MessageHeader* other; + TypedData_Get_Struct(_self, MessageHeader, &Message_type, self); TypedData_Get_Struct(_other, MessageHeader, &Message_type, other); if (self->descriptor != other->descriptor) { @@ -318,9 +342,10 @@ VALUE Message_hash(VALUE _self) { */ VALUE Message_inspect(VALUE _self) { MessageHeader* self; + VALUE str; TypedData_Get_Struct(_self, MessageHeader, &Message_type, self); - VALUE str = rb_str_new2("<"); + str = rb_str_new2("<"); str = rb_str_append(str, rb_str_new2(rb_class2name(CLASS_OF(_self)))); str = rb_str_cat2(str, ": "); str = rb_str_append(str, layout_inspect( @@ -332,11 +357,12 @@ VALUE Message_inspect(VALUE _self) { VALUE Message_to_h(VALUE _self) { MessageHeader* self; + VALUE hash; + upb_msg_field_iter it; TypedData_Get_Struct(_self, MessageHeader, &Message_type, self); - VALUE hash = rb_hash_new(); + hash = rb_hash_new(); - upb_msg_field_iter it; for (upb_msg_field_begin(&it, self->descriptor->msgdef); !upb_msg_field_done(&it); upb_msg_field_next(&it)) { @@ -363,10 +389,10 @@ VALUE Message_to_h(VALUE _self) { */ VALUE Message_index(VALUE _self, VALUE field_name) { MessageHeader* self; + const upb_fielddef* field; TypedData_Get_Struct(_self, MessageHeader, &Message_type, self); Check_Type(field_name, T_STRING); - const upb_fielddef* field = - upb_msgdef_ntofz(self->descriptor->msgdef, RSTRING_PTR(field_name)); + field = upb_msgdef_ntofz(self->descriptor->msgdef, RSTRING_PTR(field_name)); if (field == NULL) { return Qnil; } @@ -382,10 +408,10 @@ VALUE Message_index(VALUE _self, VALUE field_name) { */ VALUE Message_index_set(VALUE _self, VALUE field_name, VALUE value) { MessageHeader* self; + const upb_fielddef* field; TypedData_Get_Struct(_self, MessageHeader, &Message_type, self); Check_Type(field_name, T_STRING); - const upb_fielddef* field = - upb_msgdef_ntofz(self->descriptor->msgdef, RSTRING_PTR(field_name)); + field = upb_msgdef_ntofz(self->descriptor->msgdef, RSTRING_PTR(field_name)); if (field == NULL) { rb_raise(rb_eArgError, "Unknown field: %s", RSTRING_PTR(field_name)); } @@ -405,6 +431,9 @@ VALUE Message_descriptor(VALUE klass) { } VALUE build_class_from_descriptor(Descriptor* desc) { + const char *name; + VALUE klass; + if (desc->layout == NULL) { desc->layout = create_layout(desc->msgdef); } @@ -412,12 +441,12 @@ VALUE build_class_from_descriptor(Descriptor* desc) { desc->fill_method = new_fillmsg_decodermethod(desc, &desc->fill_method); } - const char* name = upb_msgdef_fullname(desc->msgdef); + name = upb_msgdef_fullname(desc->msgdef); if (name == NULL) { rb_raise(rb_eRuntimeError, "Descriptor does not have assigned name."); } - VALUE klass = rb_define_class_id( + klass = rb_define_class_id( // Docs say this parameter is ignored. User will assign return value to // their own toplevel constant class name. rb_intern("Message"), diff --git a/ruby/ext/google/protobuf_c/protobuf.c b/ruby/ext/google/protobuf_c/protobuf.c index d0625a10..32e3aea3 100644 --- a/ruby/ext/google/protobuf_c/protobuf.c +++ b/ruby/ext/google/protobuf_c/protobuf.c @@ -80,10 +80,11 @@ ID descriptor_instancevar_interned; // This must be named "Init_protobuf_c" because the Ruby module is named // "protobuf_c" -- the VM looks for this symbol in our .so. void Init_protobuf_c() { - descriptor_instancevar_interned = rb_intern(kDescriptorInstanceVar); VALUE google = rb_define_module("Google"); VALUE protobuf = rb_define_module_under(google, "Protobuf"); VALUE internal = rb_define_module_under(protobuf, "Internal"); + + descriptor_instancevar_interned = rb_intern(kDescriptorInstanceVar); DescriptorPool_register(protobuf); Descriptor_register(protobuf); FieldDescriptor_register(protobuf); diff --git a/ruby/ext/google/protobuf_c/repeated_field.c b/ruby/ext/google/protobuf_c/repeated_field.c index ffa60c15..72687f27 100644 --- a/ruby/ext/google/protobuf_c/repeated_field.c +++ b/ruby/ext/google/protobuf_c/repeated_field.c @@ -47,6 +47,10 @@ RepeatedField* ruby_to_RepeatedField(VALUE _self) { return self; } +void* RepeatedField_memoryat(RepeatedField* self, int index, int element_size) { + return ((uint8_t *)self->elements) + index * element_size; +} + static int index_position(VALUE _index, RepeatedField* repeated_field) { int index = NUM2INT(_index); if (index < 0 && repeated_field->size > 0) { @@ -113,16 +117,15 @@ VALUE RepeatedField_index(int argc, VALUE* argv, VALUE _self) { if (argc == 1){ if (FIXNUM_P(arg)) { /* standard case */ + void* memory; int index = index_position(argv[0], self); if (index < 0 || index >= self->size) { return Qnil; } - void* memory = (void *) (((uint8_t *)self->elements) + - index * element_size); + memory = RepeatedField_memoryat(self, index, element_size); return native_slot_get(field_type, field_type_class, memory); }else{ /* check if idx is Range */ - size_t off; switch (rb_range_beg_len(arg, &beg, &len, self->size, 0)) { case Qfalse: break; @@ -157,23 +160,24 @@ VALUE RepeatedField_index_set(VALUE _self, VALUE _index, VALUE val) { upb_fieldtype_t field_type = self->field_type; VALUE field_type_class = self->field_type_class; int element_size = native_slot_size(field_type); + void* memory; int index = index_position(_index, self); if (index < 0 || index >= (INT_MAX - 1)) { return Qnil; } if (index >= self->size) { - RepeatedField_reserve(self, index + 1); upb_fieldtype_t field_type = self->field_type; int element_size = native_slot_size(field_type); + RepeatedField_reserve(self, index + 1); for (int i = self->size; i <= index; i++) { - void* elem = (void *)(((uint8_t *)self->elements) + i * element_size); + void* elem = RepeatedField_memoryat(self, i, element_size); native_slot_init(field_type, elem); } self->size = index + 1; } - void* memory = (void *) (((uint8_t *)self->elements) + index * element_size); + memory = RepeatedField_memoryat(self, index, element_size); native_slot_set(field_type, field_type_class, memory, val); return Qnil; } @@ -181,6 +185,8 @@ VALUE RepeatedField_index_set(VALUE _self, VALUE _index, VALUE val) { static int kInitialSize = 8; void RepeatedField_reserve(RepeatedField* self, int new_size) { + void* old_elems = self->elements; + int elem_size = native_slot_size(self->field_type); if (new_size <= self->capacity) { return; } @@ -190,8 +196,6 @@ void RepeatedField_reserve(RepeatedField* self, int new_size) { while (self->capacity < new_size) { self->capacity *= 2; } - void* old_elems = self->elements; - int elem_size = native_slot_size(self->field_type); self->elements = ALLOC_N(uint8_t, elem_size * self->capacity); if (old_elems != NULL) { memcpy(self->elements, old_elems, self->size * elem_size); @@ -209,11 +213,12 @@ VALUE RepeatedField_push(VALUE _self, VALUE val) { RepeatedField* self = ruby_to_RepeatedField(_self); upb_fieldtype_t field_type = self->field_type; int element_size = native_slot_size(field_type); + void* memory; + RepeatedField_reserve(self, self->size + 1); - int index = self->size; - void* memory = (void *) (((uint8_t *)self->elements) + index * element_size); + memory = (void *) (((uint8_t *)self->elements) + self->size * element_size); native_slot_set(field_type, self->field_type_class, memory, val); - // native_slot_set may raise an error; bump index only after set. + // native_slot_set may raise an error; bump size only after set. self->size++; return _self; } @@ -224,9 +229,10 @@ void RepeatedField_push_native(VALUE _self, void* data) { RepeatedField* self = ruby_to_RepeatedField(_self); upb_fieldtype_t field_type = self->field_type; int element_size = native_slot_size(field_type); + void* memory; + RepeatedField_reserve(self, self->size + 1); - int index = self->size; - void* memory = (void *) (((uint8_t *)self->elements) + index * element_size); + memory = (void *) (((uint8_t *)self->elements) + self->size * element_size); memcpy(memory, data, element_size); self->size++; } @@ -235,7 +241,7 @@ void* RepeatedField_index_native(VALUE _self, int index) { RepeatedField* self = ruby_to_RepeatedField(_self); upb_fieldtype_t field_type = self->field_type; int element_size = native_slot_size(field_type); - return ((uint8_t *)self->elements) + index * element_size; + return RepeatedField_memoryat(self, index, element_size); } /* @@ -246,12 +252,16 @@ VALUE RepeatedField_pop_one(VALUE _self) { upb_fieldtype_t field_type = self->field_type; VALUE field_type_class = self->field_type_class; int element_size = native_slot_size(field_type); + int index; + void* memory; + VALUE ret; + if (self->size == 0) { return Qnil; } - int index = self->size - 1; - void* memory = (void *) (((uint8_t *)self->elements) + index * element_size); - VALUE ret = native_slot_get(field_type, field_type_class, memory); + index = self->size - 1; + memory = RepeatedField_memoryat(self, index, element_size); + ret = native_slot_get(field_type, field_type_class, memory); self->size--; return ret; } @@ -320,10 +330,10 @@ VALUE RepeatedField_dup(VALUE _self) { RepeatedField* self = ruby_to_RepeatedField(_self); VALUE new_rptfield = RepeatedField_new_this_type(_self); RepeatedField* new_rptfield_self = ruby_to_RepeatedField(new_rptfield); - RepeatedField_reserve(new_rptfield_self, self->size); upb_fieldtype_t field_type = self->field_type; size_t elem_size = native_slot_size(field_type); size_t off = 0; + RepeatedField_reserve(new_rptfield_self, self->size); for (int i = 0; i < self->size; i++, off += elem_size) { void* to_mem = (uint8_t *)new_rptfield_self->elements + off; void* from_mem = (uint8_t *)self->elements + off; @@ -339,10 +349,10 @@ VALUE RepeatedField_deep_copy(VALUE _self) { RepeatedField* self = ruby_to_RepeatedField(_self); VALUE new_rptfield = RepeatedField_new_this_type(_self); RepeatedField* new_rptfield_self = ruby_to_RepeatedField(new_rptfield); - RepeatedField_reserve(new_rptfield_self, self->size); upb_fieldtype_t field_type = self->field_type; size_t elem_size = native_slot_size(field_type); size_t off = 0; + RepeatedField_reserve(new_rptfield_self, self->size); for (int i = 0; i < self->size; i++, off += elem_size) { void* to_mem = (uint8_t *)new_rptfield_self->elements + off; void* from_mem = (uint8_t *)self->elements + off; @@ -389,34 +399,39 @@ VALUE RepeatedField_to_ary(VALUE _self) { * indicated that every element has equal value. */ VALUE RepeatedField_eq(VALUE _self, VALUE _other) { + RepeatedField* self; + RepeatedField* other; + if (_self == _other) { return Qtrue; } - RepeatedField* self = ruby_to_RepeatedField(_self); if (TYPE(_other) == T_ARRAY) { VALUE self_ary = RepeatedField_to_ary(_self); return rb_equal(self_ary, _other); } - RepeatedField* other = ruby_to_RepeatedField(_other); + self = ruby_to_RepeatedField(_self); + other = ruby_to_RepeatedField(_other); if (self->field_type != other->field_type || self->field_type_class != other->field_type_class || self->size != other->size) { return Qfalse; } - upb_fieldtype_t field_type = self->field_type; - size_t elem_size = native_slot_size(field_type); - size_t off = 0; - for (int i = 0; i < self->size; i++, off += elem_size) { - void* self_mem = ((uint8_t *)self->elements) + off; - void* other_mem = ((uint8_t *)other->elements) + off; - if (!native_slot_eq(field_type, self_mem, other_mem)) { - return Qfalse; + { + upb_fieldtype_t field_type = self->field_type; + size_t elem_size = native_slot_size(field_type); + size_t off = 0; + for (int i = 0; i < self->size; i++, off += elem_size) { + void* self_mem = ((uint8_t *)self->elements) + off; + void* other_mem = ((uint8_t *)other->elements) + off; + if (!native_slot_eq(field_type, self_mem, other_mem)) { + return Qfalse; + } } + return Qtrue; } - return Qtrue; } /* @@ -488,7 +503,6 @@ VALUE RepeatedField_plus(VALUE _self, VALUE list) { * concats the passed in array to self. Returns a Ruby array. */ VALUE RepeatedField_concat(VALUE _self, VALUE list) { - RepeatedField* self = ruby_to_RepeatedField(_self); Check_Type(list, T_ARRAY); for (int i = 0; i < RARRAY_LEN(list); i++) { RepeatedField_push(_self, rb_ary_entry(list, i)); @@ -564,9 +578,9 @@ void RepeatedField_init_args(int argc, VALUE* argv, void RepeatedField_mark(void* _self) { RepeatedField* self = (RepeatedField*)_self; - rb_gc_mark(self->field_type_class); upb_fieldtype_t field_type = self->field_type; int element_size = native_slot_size(field_type); + rb_gc_mark(self->field_type_class); for (int i = 0; i < self->size; i++) { void* memory = (((uint8_t *)self->elements) + i * element_size); native_slot_mark(self->field_type, memory); @@ -592,12 +606,12 @@ void RepeatedField_free(void* _self) { */ VALUE RepeatedField_alloc(VALUE klass) { RepeatedField* self = ALLOC(RepeatedField); + VALUE ret = TypedData_Wrap_Struct(klass, &RepeatedField_type, self); self->elements = NULL; self->size = 0; self->capacity = 0; self->field_type = -1; self->field_type_class = Qnil; - VALUE ret = TypedData_Wrap_Struct(klass, &RepeatedField_type, self); return ret; } diff --git a/ruby/ext/google/protobuf_c/storage.c b/ruby/ext/google/protobuf_c/storage.c index c4cdc9cc..b1f65f41 100644 --- a/ruby/ext/google/protobuf_c/storage.c +++ b/ruby/ext/google/protobuf_c/storage.c @@ -166,11 +166,11 @@ void native_slot_set_value_and_case(upb_fieldtype_t type, VALUE type_class, break; } case UPB_TYPE_ENUM: { + int32_t int_val = 0; if (!is_ruby_num(value) && TYPE(value) != T_SYMBOL) { rb_raise(rb_eTypeError, "Expected number or symbol type for enum field."); } - int32_t int_val = 0; if (TYPE(value) == T_SYMBOL) { // Ensure that the given symbol exists in the enum module. VALUE lookup = rb_funcall(type_class, rb_intern("resolve"), 1, value); @@ -346,24 +346,33 @@ bool native_slot_eq(upb_fieldtype_t type, void* mem1, void* mem2) { // Map field utilities. // ----------------------------------------------------------------------------- -bool is_map_field(const upb_fielddef* field) { +const upb_msgdef* tryget_map_entry_msgdef(const upb_fielddef* field) { + const upb_msgdef* subdef; if (upb_fielddef_label(field) != UPB_LABEL_REPEATED || upb_fielddef_type(field) != UPB_TYPE_MESSAGE) { - return false; + return NULL; } - const upb_msgdef* subdef = upb_fielddef_msgsubdef(field); - return upb_msgdef_mapentry(subdef); + subdef = upb_fielddef_msgsubdef(field); + return upb_msgdef_mapentry(subdef) ? subdef : NULL; +} + +const upb_msgdef *map_entry_msgdef(const upb_fielddef* field) { + const upb_msgdef* subdef = tryget_map_entry_msgdef(field); + assert(subdef); + return subdef; +} + +bool is_map_field(const upb_fielddef *field) { + return tryget_map_entry_msgdef(field) != NULL; } const upb_fielddef* map_field_key(const upb_fielddef* field) { - assert(is_map_field(field)); - const upb_msgdef* subdef = upb_fielddef_msgsubdef(field); + const upb_msgdef* subdef = map_entry_msgdef(field); return map_entry_key(subdef); } const upb_fielddef* map_field_value(const upb_fielddef* field) { - assert(is_map_field(field)); - const upb_msgdef* subdef = upb_fielddef_msgsubdef(field); + const upb_msgdef* subdef = map_entry_msgdef(field); return map_entry_value(subdef); } @@ -391,14 +400,17 @@ static size_t align_up_to(size_t offset, size_t granularity) { MessageLayout* create_layout(const upb_msgdef* msgdef) { MessageLayout* layout = ALLOC(MessageLayout); int nfields = upb_msgdef_numfields(msgdef); - layout->fields = ALLOC_N(MessageField, nfields); - upb_msg_field_iter it; + upb_msg_oneof_iter oit; size_t off = 0; + + layout->fields = ALLOC_N(MessageField, nfields); + for (upb_msg_field_begin(&it, msgdef); !upb_msg_field_done(&it); upb_msg_field_next(&it)) { const upb_fielddef* field = upb_msg_iter_field(&it); + size_t field_size; if (upb_fielddef_containingoneof(field)) { // Oneofs are handled separately below. @@ -406,7 +418,7 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { } // Allocate |field_size| bytes for this field in the layout. - size_t field_size = 0; + field_size = 0; if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { field_size = sizeof(VALUE); } else { @@ -433,11 +445,11 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { // members (8 or 16 bits respectively), so conceivably we could assign // consecutive case numbers and then pick a smaller oneof case slot size, but // the complexity to implement this indirection is probably not worthwhile. - upb_msg_oneof_iter oit; for (upb_msg_oneof_begin(&oit, msgdef); !upb_msg_oneof_done(&oit); upb_msg_oneof_next(&oit)) { const upb_oneofdef* oneof = upb_msg_iter_oneof(&oit); + upb_oneof_iter fit; // Always allocate NATIVE_SLOT_MAX_SIZE bytes, but share the slot between // all fields. @@ -445,7 +457,6 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { // Align the offset. off = align_up_to(off, field_size); // Assign all fields in the oneof this same offset. - upb_oneof_iter fit; for (upb_oneof_begin(&fit, oneof); !upb_oneof_done(&fit); upb_oneof_next(&fit)) { @@ -460,12 +471,12 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { !upb_msg_oneof_done(&oit); upb_msg_oneof_next(&oit)) { const upb_oneofdef* oneof = upb_msg_iter_oneof(&oit); + upb_oneof_iter fit; size_t field_size = sizeof(uint32_t); // Align the offset. off = (off + field_size - 1) & ~(field_size - 1); // Assign all fields in the oneof this same offset. - upb_oneof_iter fit; for (upb_oneof_begin(&fit, oneof); !upb_oneof_done(&fit); upb_oneof_next(&fit)) { @@ -541,6 +552,7 @@ VALUE layout_get(MessageLayout* layout, } static void check_repeated_field_type(VALUE val, const upb_fielddef* field) { + RepeatedField* self; assert(upb_fielddef_label(field) == UPB_LABEL_REPEATED); if (!RB_TYPE_P(val, T_DATA) || !RTYPEDDATA_P(val) || @@ -548,7 +560,7 @@ static void check_repeated_field_type(VALUE val, const upb_fielddef* field) { rb_raise(rb_eTypeError, "Expected repeated field array"); } - RepeatedField* self = ruby_to_RepeatedField(val); + self = ruby_to_RepeatedField(val); if (self->field_type != upb_fielddef_type(field)) { rb_raise(rb_eTypeError, "Repeated field array has wrong element type"); } @@ -564,16 +576,16 @@ static void check_repeated_field_type(VALUE val, const upb_fielddef* field) { } static void check_map_field_type(VALUE val, const upb_fielddef* field) { - assert(is_map_field(field)); const upb_fielddef* key_field = map_field_key(field); const upb_fielddef* value_field = map_field_value(field); + Map* self; if (!RB_TYPE_P(val, T_DATA) || !RTYPEDDATA_P(val) || RTYPEDDATA_TYPE(val) != &Map_type) { rb_raise(rb_eTypeError, "Expected Map instance"); } - Map* self = ruby_to_Map(val); + self = ruby_to_Map(val); if (self->key_type != upb_fielddef_type(key_field)) { rb_raise(rb_eTypeError, "Map key type does not match field's key type"); } -- cgit v1.2.3 From d61e6adfcc82d4a1b3b3882f07b39c77b42eafc1 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Thu, 20 Aug 2015 16:41:32 -0700 Subject: Return TypedData_Wrap_Struct directly. Change-Id: I6cf77f01370204ad4bc7b345a040a9a3de1706a0 --- ruby/ext/google/protobuf_c/map.c | 3 +-- ruby/ext/google/protobuf_c/repeated_field.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) (limited to 'ruby/ext') diff --git a/ruby/ext/google/protobuf_c/map.c b/ruby/ext/google/protobuf_c/map.c index 5043f395..92fc7286 100644 --- a/ruby/ext/google/protobuf_c/map.c +++ b/ruby/ext/google/protobuf_c/map.c @@ -167,10 +167,9 @@ void Map_free(void* _self) { VALUE Map_alloc(VALUE klass) { Map* self = ALLOC(Map); - VALUE ret = TypedData_Wrap_Struct(klass, &Map_type, self); memset(self, 0, sizeof(Map)); self->value_type_class = Qnil; - return ret; + return TypedData_Wrap_Struct(klass, &Map_type, self); } static bool needs_typeclass(upb_fieldtype_t type) { diff --git a/ruby/ext/google/protobuf_c/repeated_field.c b/ruby/ext/google/protobuf_c/repeated_field.c index 72687f27..83afbc91 100644 --- a/ruby/ext/google/protobuf_c/repeated_field.c +++ b/ruby/ext/google/protobuf_c/repeated_field.c @@ -606,13 +606,12 @@ void RepeatedField_free(void* _self) { */ VALUE RepeatedField_alloc(VALUE klass) { RepeatedField* self = ALLOC(RepeatedField); - VALUE ret = TypedData_Wrap_Struct(klass, &RepeatedField_type, self); self->elements = NULL; self->size = 0; self->capacity = 0; self->field_type = -1; self->field_type_class = Qnil; - return ret; + return TypedData_Wrap_Struct(klass, &RepeatedField_type, self); } VALUE RepeatedField_init(int argc, VALUE* argv, VALUE self) { -- cgit v1.2.3