Browse Source

Addressed code-review comments.

Chris Fallin 10 years ago
parent
commit
9de35e7421

+ 2 - 2
ruby/ext/google/protobuf_c/defs.c

@@ -58,11 +58,11 @@ static upb_def* check_notfrozen(const upb_def* def) {
 }
 }
 
 
 static upb_msgdef* check_msg_notfrozen(const upb_msgdef* def) {
 static upb_msgdef* check_msg_notfrozen(const upb_msgdef* def) {
-  return (upb_msgdef*)check_notfrozen((const upb_def*)def);
+  return upb_downcast_msgdef_mutable(check_notfrozen((const upb_def*)def));
 }
 }
 
 
 static upb_fielddef* check_field_notfrozen(const upb_fielddef* def) {
 static upb_fielddef* check_field_notfrozen(const upb_fielddef* def) {
-  return (upb_fielddef*)check_notfrozen((const upb_def*)def);
+  return upb_downcast_fielddef_mutable(check_notfrozen((const upb_def*)def));
 }
 }
 
 
 static upb_oneofdef* check_oneof_notfrozen(const upb_oneofdef* def) {
 static upb_oneofdef* check_oneof_notfrozen(const upb_oneofdef* def) {

+ 3 - 3
ruby/ext/google/protobuf_c/encode_decode.c

@@ -558,11 +558,11 @@ static void add_handlers_for_message(const void *closure, upb_handlers *h) {
     const upb_fielddef *f = upb_msg_iter_field(&i);
     const upb_fielddef *f = upb_msg_iter_field(&i);
     size_t offset = desc->layout->fields[upb_fielddef_index(f)].offset +
     size_t offset = desc->layout->fields[upb_fielddef_index(f)].offset +
         sizeof(MessageHeader);
         sizeof(MessageHeader);
-    size_t oneof_case_offset =
-        desc->layout->fields[upb_fielddef_index(f)].case_offset +
-        sizeof(MessageHeader);
 
 
     if (upb_fielddef_containingoneof(f)) {
     if (upb_fielddef_containingoneof(f)) {
+      size_t oneof_case_offset =
+          desc->layout->fields[upb_fielddef_index(f)].case_offset +
+          sizeof(MessageHeader);
       add_handlers_for_oneof_field(h, f, offset, oneof_case_offset);
       add_handlers_for_oneof_field(h, f, offset, oneof_case_offset);
     } else if (is_map_field(f)) {
     } else if (is_map_field(f)) {
       add_handlers_for_mapfield(h, f, offset, desc);
       add_handlers_for_mapfield(h, f, offset, desc);

+ 7 - 2
ruby/ext/google/protobuf_c/storage.c

@@ -366,6 +366,11 @@ const upb_fielddef* map_entry_value(const upb_msgdef* msgdef) {
 // Memory layout management.
 // Memory layout management.
 // -----------------------------------------------------------------------------
 // -----------------------------------------------------------------------------
 
 
+static size_t align_up_to(size_t offset, size_t granularity) {
+  // Granularity must be a power of two.
+  return (offset + granularity - 1) & ~(granularity - 1);
+}
+
 MessageLayout* create_layout(const upb_msgdef* msgdef) {
 MessageLayout* create_layout(const upb_msgdef* msgdef) {
   MessageLayout* layout = ALLOC(MessageLayout);
   MessageLayout* layout = ALLOC(MessageLayout);
   int nfields = upb_msgdef_numfields(msgdef);
   int nfields = upb_msgdef_numfields(msgdef);
@@ -391,7 +396,7 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) {
       field_size = native_slot_size(upb_fielddef_type(field));
       field_size = native_slot_size(upb_fielddef_type(field));
     }
     }
     // Align current offset up to |size| granularity.
     // Align current offset up to |size| granularity.
-    off = (off + field_size - 1) & ~(field_size - 1);
+    off = align_up_to(off, field_size);
     layout->fields[upb_fielddef_index(field)].offset = off;
     layout->fields[upb_fielddef_index(field)].offset = off;
     layout->fields[upb_fielddef_index(field)].case_offset = MESSAGE_FIELD_NO_CASE;
     layout->fields[upb_fielddef_index(field)].case_offset = MESSAGE_FIELD_NO_CASE;
     off += field_size;
     off += field_size;
@@ -413,7 +418,7 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) {
     // all fields.
     // all fields.
     size_t field_size = NATIVE_SLOT_MAX_SIZE;
     size_t field_size = NATIVE_SLOT_MAX_SIZE;
     // Align the offset.
     // Align the offset.
-    off = (off + field_size - 1) & ~(field_size - 1);
+    off = align_up_to(off, field_size);
     // Assign all fields in the oneof this same offset.
     // Assign all fields in the oneof this same offset.
     upb_oneof_iter fit;
     upb_oneof_iter fit;
     for (upb_oneof_begin(&fit, oneof);
     for (upb_oneof_begin(&fit, oneof);