From 2b77da3da8234484ebc099c560671ea21ab7181b Mon Sep 17 00:00:00 2001
From: Josh Haberman <jhaberman@gmail.com>
Date: Wed, 7 Dec 2016 17:02:48 -0800
Subject: Update for final PR comments.

---
 upb/msg.c | 24 ++++++++++--------------
 upb/msg.h |  5 ++++-
 2 files changed, 14 insertions(+), 15 deletions(-)

(limited to 'upb')

diff --git a/upb/msg.c b/upb/msg.c
index 36dcf76..1a1066a 100644
--- a/upb/msg.c
+++ b/upb/msg.c
@@ -152,7 +152,7 @@ struct upb_msglayout {
   size_t size;
   size_t extdict_offset;
   void *default_msg;
-  uint32_t *offsets;
+  uint32_t *field_offsets;
   uint32_t *case_offsets;
   uint32_t *hasbits;
   bool has_extdict;
@@ -184,7 +184,7 @@ static size_t upb_msglayout_place(upb_msglayout *l, size_t size) {
 
 static uint32_t upb_msglayout_offset(const upb_msglayout *l,
                                      const upb_fielddef *f) {
-  return l->offsets[upb_fielddef_index(f)];
+  return l->field_offsets[upb_fielddef_index(f)];
 }
 
 static uint32_t upb_msglayout_hasbit(const upb_msglayout *l,
@@ -242,8 +242,8 @@ static upb_msglayout *upb_msglayout_new(const upb_msgdef *m) {
 
   l->msgdef = m;
   l->align = 1;
-  l->offsets = (uint32_t*)CHARPTR_AT(l, sizeof(*l));
-  l->case_offsets = l->offsets + upb_msgdef_numfields(m);
+  l->field_offsets = (uint32_t*)CHARPTR_AT(l, sizeof(*l));
+  l->case_offsets = l->field_offsets + upb_msgdef_numfields(m);
   l->hasbits = l->case_offsets + upb_msgdef_numoneofs(m);
 
   /* Allocate data offsets in three stages:
@@ -274,13 +274,15 @@ static upb_msglayout *upb_msglayout_new(const upb_msgdef *m) {
        upb_msg_field_next(&it)) {
     const upb_fielddef* f = upb_msg_iter_field(&it);
     size_t field_size = upb_msg_fieldsize(f);
+    size_t index = upb_fielddef_index(f);
+
 
     if (upb_fielddef_containingoneof(f)) {
       /* Oneofs are handled separately below. */
       continue;
     }
 
-    l->offsets[upb_fielddef_index(f)] = upb_msglayout_place(l, field_size);
+    l->field_offsets[index] = upb_msglayout_place(l, field_size);
   }
 
   /* Allocate oneof fields.  Each oneof field consists of a uint32 for the case
@@ -312,7 +314,7 @@ static upb_msglayout *upb_msglayout_new(const upb_msgdef *m) {
     for (upb_oneof_begin(&fit, oneof); !upb_oneof_done(&fit);
          upb_oneof_next(&fit)) {
       const upb_fielddef* f = upb_oneof_iter_field(&fit);
-      l->offsets[upb_fielddef_index(f)] = val_offset;
+      l->field_offsets[upb_fielddef_index(f)] = val_offset;
     }
   }
 
@@ -372,12 +374,6 @@ const upb_symtab *upb_msgfactory_symtab(const upb_msgfactory *f) {
   return f->symtab;
 }
 
-/* Requires:
- * - m is in upb_msgfactory_symtab(f)
- * - upb_msgdef_mapentry(m) == false (since map messages can't have layouts).
- *
- * The returned layout will live for as long as the msgfactory does.
- */
 const upb_msglayout *upb_msgfactory_getlayout(upb_msgfactory *f,
                                               const upb_msgdef *m) {
   upb_value v;
@@ -615,7 +611,7 @@ upb_msgval upb_msg_get(const upb_msg *msg, const upb_fielddef *f,
       return upb_msgval_fromdefault(f);
     }
   } else {
-    size_t ofs = l->offsets[upb_fielddef_index(f)];
+    size_t ofs = l->field_offsets[upb_fielddef_index(f)];
     const upb_oneofdef *o = upb_fielddef_containingoneof(f);
     upb_msgval ret;
 
@@ -646,7 +642,7 @@ bool upb_msg_set(upb_msg *msg,
       return false;
     }
   } else {
-    size_t ofs = l->offsets[upb_fielddef_index(f)];
+    size_t ofs = l->field_offsets[upb_fielddef_index(f)];
     const upb_oneofdef *o = upb_fielddef_containingoneof(f);
 
     if (o) {
diff --git a/upb/msg.h b/upb/msg.h
index 12681a8..11f0d5c 100644
--- a/upb/msg.h
+++ b/upb/msg.h
@@ -109,7 +109,10 @@ const upb_symtab *upb_msgfactory_symtab(const upb_msgfactory *f);
  * - m is in upb_msgfactory_symtab(f)
  * - upb_msgdef_mapentry(m) == false (since map messages can't have layouts).
  *
- * The returned objects will live for as long as the msgfactory does. */
+ * The returned objects will live for as long as the msgfactory does.
+ *
+ * TODO(haberman): consider making this thread-safe and take a const
+ * upb_msgfactory. */
 const upb_msglayout *upb_msgfactory_getlayout(upb_msgfactory *f,
                                               const upb_msgdef *m);
 const upb_handlers *upb_msgfactory_getmergehandlers(upb_msgfactory *f,
-- 
cgit v1.2.3