From 50b360f97414889005cb52b7f1cc4d76c819f470 Mon Sep 17 00:00:00 2001
From: Joshua Haberman <jhaberman@gmail.com>
Date: Wed, 20 Oct 2021 16:17:43 -0700
Subject: [PATCH] Addressed PR comments.

---
 upb/util/required_fields.c | 20 +++++++++++++-------
 upb/util/required_fields.h |  2 ++
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/upb/util/required_fields.c b/upb/util/required_fields.c
index e854ef3fa3..9315796c70 100644
--- a/upb/util/required_fields.c
+++ b/upb/util/required_fields.c
@@ -193,13 +193,9 @@ static void upb_FindContext_Pop(upb_FindContext* ctx) {
   ctx->stack.size--;
 }
 
-static void upb_util_FindUnsetRequiredInternal(upb_FindContext* ctx,
-                                               const upb_msg* msg,
-                                               const upb_msgdef* m) {
-  // OPT: add markers in the schema for where we can avoid iterating:
-  // 1. messages with no required fields.
-  // 2. messages that cannot possibly reach any required fields.
-
+static void upb_util_FindUnsetInMessage(upb_FindContext* ctx,
+                                        const upb_msg* msg,
+                                        const upb_msgdef* m) {
   // Iterate over all fields to see if any required fields are missing.
   for (int i = 0, n = upb_msgdef_fieldcount(m); i < n; i++) {
     const upb_fielddef* f = upb_msgdef_field(m, i);
@@ -223,6 +219,16 @@ static void upb_util_FindUnsetRequiredInternal(upb_FindContext* ctx,
       }
     }
   }
+}
+
+static void upb_util_FindUnsetRequiredInternal(upb_FindContext* ctx,
+                                               const upb_msg* msg,
+                                               const upb_msgdef* m) {
+  // OPT: add markers in the schema for where we can avoid iterating:
+  // 1. messages with no required fields.
+  // 2. messages that cannot possibly reach any required fields.
+
+  upb_util_FindUnsetInMessage(ctx, msg, m);
 
   // Iterate over all present fields to find sub-messages that might be missing
   // required fields.  This may revisit some of the fields already inspected
diff --git a/upb/util/required_fields.h b/upb/util/required_fields.h
index f934f184a4..8b1dd6c366 100644
--- a/upb/util/required_fields.h
+++ b/upb/util/required_fields.h
@@ -67,6 +67,8 @@ typedef union {
 //
 // The output buffer `buf` will always be NULL-terminated. If the output data
 // (including NULL terminator) exceeds `size`, the result will be truncated.
+// Returns the string length of the data we attempted to write, excluding the
+// terminating NULL.
 size_t upb_FieldPath_ToText(upb_FieldPathEntry **path, char *buf, size_t size);
 
 // Checks whether `msg` or any of its children has unset required fields,