From 9de35e742167b1dd2941cefab6e99239207f8e2c Mon Sep 17 00:00:00 2001
From: Chris Fallin <cfallin@google.com>
Date: Mon, 26 Jan 2015 11:23:19 -0800
Subject: Addressed code-review comments.

---
 ruby/ext/google/protobuf_c/defs.c          | 4 ++--
 ruby/ext/google/protobuf_c/encode_decode.c | 6 +++---
 ruby/ext/google/protobuf_c/storage.c       | 9 +++++++--
 3 files changed, 12 insertions(+), 7 deletions(-)

(limited to 'ruby/ext/google')

diff --git a/ruby/ext/google/protobuf_c/defs.c b/ruby/ext/google/protobuf_c/defs.c
index 446a5586..b39c27f4 100644
--- a/ruby/ext/google/protobuf_c/defs.c
+++ b/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) {
-  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) {
-  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) {
diff --git a/ruby/ext/google/protobuf_c/encode_decode.c b/ruby/ext/google/protobuf_c/encode_decode.c
index 0db86209..3a441be3 100644
--- a/ruby/ext/google/protobuf_c/encode_decode.c
+++ b/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);
     size_t offset = desc->layout->fields[upb_fielddef_index(f)].offset +
         sizeof(MessageHeader);
-    size_t oneof_case_offset =
-        desc->layout->fields[upb_fielddef_index(f)].case_offset +
-        sizeof(MessageHeader);
 
     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);
     } else if (is_map_field(f)) {
       add_handlers_for_mapfield(h, f, offset, desc);
diff --git a/ruby/ext/google/protobuf_c/storage.c b/ruby/ext/google/protobuf_c/storage.c
index adb74394..fac06e0a 100644
--- a/ruby/ext/google/protobuf_c/storage.c
+++ b/ruby/ext/google/protobuf_c/storage.c
@@ -366,6 +366,11 @@ const upb_fielddef* map_entry_value(const upb_msgdef* msgdef) {
 // 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* layout = ALLOC(MessageLayout);
   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));
     }
     // 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)].case_offset = MESSAGE_FIELD_NO_CASE;
     off += field_size;
@@ -413,7 +418,7 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) {
     // all fields.
     size_t field_size = NATIVE_SLOT_MAX_SIZE;
     // 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.
     upb_oneof_iter fit;
     for (upb_oneof_begin(&fit, oneof);
-- 
cgit v1.2.3