From 28268113678e869ffa42eb20cab0c338f55142dc Mon Sep 17 00:00:00 2001
From: Joshua Haberman <jhaberman@gmail.com>
Date: Mon, 10 Jul 2017 16:43:55 -0500
Subject: Responded to PR comments.

---
 upb/msg.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/upb/msg.c b/upb/msg.c
index 5574363..5729408 100644
--- a/upb/msg.c
+++ b/upb/msg.c
@@ -206,6 +206,7 @@ static bool upb_msglayout_initdefault(upb_msglayout *l, const upb_msgdef *m) {
         continue;
       }
 
+      /* TODO(haberman): handle strings. */
       if (!upb_fielddef_isstring(f) &&
           !upb_fielddef_issubmsg(f) &&
           !upb_fielddef_isseq(f)) {
@@ -244,11 +245,21 @@ static upb_msglayout *upb_msglayout_new(const upb_msgdef *m) {
 
   memset(l, 0, sizeof(*l));
 
-  /* TODO(haberman): check OOM. */
   fields = upb_gmalloc(upb_msgdef_numfields(m) * sizeof(*fields));
   submsgs = upb_gmalloc(submsg_count * sizeof(*submsgs));
   oneofs = upb_gmalloc(upb_msgdef_numoneofs(m) * sizeof(*oneofs));
 
+  if ((!fields && upb_msgdef_numfields(m)) ||
+      (!submsgs && submsg_count) ||
+      (!oneofs && upb_msgdef_numoneofs(m))) {
+    /* OOM. */
+    upb_gfree(l);
+    upb_gfree(fields);
+    upb_gfree(submsgs);
+    upb_gfree(oneofs);
+    return NULL;
+  }
+
   l->data.field_count = upb_msgdef_numfields(m);
   l->data.oneof_count = upb_msgdef_numoneofs(m);
   l->data.fields = fields;
@@ -342,19 +353,6 @@ static upb_msglayout *upb_msglayout_new(const upb_msgdef *m) {
   }
 }
 
-upb_msglayout *upb_msglayout_frominit_v1(
-    const struct upb_msglayout_msginit_v1 *init, upb_alloc *a) {
-  UPB_UNUSED(a);
-  /* If upb upgrades to a v2, this would create a heap-allocated v2. */
-  return (upb_msglayout*)init;
-}
-
-void upb_msglayout_uninit_v1(upb_msglayout *layout, upb_alloc *a) {
-  UPB_UNUSED(layout);
-  UPB_UNUSED(a);
-  /* If upb upgrades to a v2, this would free the heap-allocated v2. */
-}
-
 
 /** upb_msgfactory ************************************************************/
 
@@ -662,8 +660,9 @@ typedef struct {
   upb_msg_internal base;
 } upb_msg_internal_withext;
 
-#define INTERNAL_MEMBERS_SIZE(l) \
-    sizeof(upb_msg_internal) - (l->data.extendable * sizeof(void*))
+static int upb_msg_internalsize(const upb_msglayout *l) {
+    return sizeof(upb_msg_internal) - l->data.extendable * sizeof(void*);
+}
 
 static upb_msg_internal *upb_msg_getinternal(upb_msg *msg) {
   return VOIDPTR_AT(msg, -sizeof(upb_msg_internal));
@@ -697,18 +696,20 @@ static uint32_t *upb_msg_oneofcase(const upb_msg *msg, int field_index,
 }
 
 size_t upb_msg_sizeof(const upb_msglayout *l) {
-  return l->data.size + INTERNAL_MEMBERS_SIZE(l);
+  return l->data.size + upb_msg_internalsize(l);
 }
 
 upb_msg *upb_msg_init(void *mem, const upb_msglayout *l, upb_alloc *a) {
-  upb_msg *msg = VOIDPTR_AT(mem, INTERNAL_MEMBERS_SIZE(l));
+  upb_msg *msg = VOIDPTR_AT(mem, upb_msg_internalsize(l));
   if (l->data.default_msg) {
     memcpy(msg, l->data.default_msg, l->data.size);
   } else {
     memset(msg, 0, l->data.size);
   }
 
+  UPB_ASSERT(!upb_msg_getinternal(msg)->alloc);
   upb_msg_getinternal(msg)->alloc = a;
+
   if (l->data.extendable) {
     upb_msg_getinternalwithext(msg, l)->extdict = NULL;
   }
@@ -725,7 +726,7 @@ void *upb_msg_uninit(upb_msg *msg, const upb_msglayout *l) {
     }
   }
 
-  return VOIDPTR_AT(msg, -INTERNAL_MEMBERS_SIZE(l));
+  return VOIDPTR_AT(msg, -upb_msg_internalsize(l));
 }
 
 upb_msg *upb_msg_new(const upb_msglayout *l, upb_alloc *a) {
-- 
cgit v1.2.3


From af43ea72b549aa36a73d691d7b562ec67f04def3 Mon Sep 17 00:00:00 2001
From: Joshua Haberman <jhaberman@gmail.com>
Date: Mon, 10 Jul 2017 21:48:28 -0500
Subject: Removed incorrect assertion.

Internal members aren't initialized by default_msg.
---
 upb/msg.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/upb/msg.c b/upb/msg.c
index 16a9dc1..129d881 100644
--- a/upb/msg.c
+++ b/upb/msg.c
@@ -705,7 +705,6 @@ upb_msg *upb_msg_init(void *mem, const upb_msglayout *l, upb_alloc *a) {
     memset(msg, 0, l->data.size);
   }
 
-  UPB_ASSERT(!upb_msg_getinternal(msg)->alloc);
   upb_msg_getinternal(msg)->alloc = a;
 
   if (l->data.extendable) {
-- 
cgit v1.2.3


From 3e8acc3f4e4a2045b04dd8794c87b2a198542382 Mon Sep 17 00:00:00 2001
From: Joshua Haberman <jhaberman@gmail.com>
Date: Mon, 10 Jul 2017 21:52:27 -0500
Subject: Removed incorrect assert and added comments.

---
 upb/msg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/upb/msg.c b/upb/msg.c
index 5729408..850e86e 100644
--- a/upb/msg.c
+++ b/upb/msg.c
@@ -701,13 +701,15 @@ size_t upb_msg_sizeof(const upb_msglayout *l) {
 
 upb_msg *upb_msg_init(void *mem, const upb_msglayout *l, upb_alloc *a) {
   upb_msg *msg = VOIDPTR_AT(mem, upb_msg_internalsize(l));
+
+  /* Initialize normal members. */
   if (l->data.default_msg) {
     memcpy(msg, l->data.default_msg, l->data.size);
   } else {
     memset(msg, 0, l->data.size);
   }
 
-  UPB_ASSERT(!upb_msg_getinternal(msg)->alloc);
+  /* Initialize internal members. */
   upb_msg_getinternal(msg)->alloc = a;
 
   if (l->data.extendable) {
-- 
cgit v1.2.3