[GitHub] [daffodil] tuxji commented on a diff in pull request #998: Add left over data check to generated C parsers

["tuxji (via GitHub)" <gi...@apache.org>]

tuxji commented on code in PR #998:
URL: https://github.com/apache/daffodil/pull/998#discussion_r1156254199


##########
daffodil-codegen-c/src/main/resources/org/apache/daffodil/codegen/c/files/libruntime/parsers.c:
##########
@@ -594,3 +594,34 @@ parse_check_bounds(const char *name, size_t count, size_t minOccurs, size_t maxO
         pstate->error = &error;
     }
 }
+
+// Check for any data left over after end of parse
+
+void
+check_pstate(PState *pstate)

Review Comment:
   No, this function probably won't do anything else, so I've renamed it accordingly to `no_leftover_data`.



##########
daffodil-codegen-c/src/main/resources/org/apache/daffodil/codegen/c/files/libruntime/parsers.c:
##########
@@ -594,3 +594,34 @@ parse_check_bounds(const char *name, size_t count, size_t minOccurs, size_t maxO
         pstate->error = &error;
     }
 }
+
+// Check for any data left over after end of parse
+
+void
+check_pstate(PState *pstate)
+{
+    // Skip the check if we already have an error
+    if (!pstate->error)
+    {
+        // Check for any unread bits left in pstate's fractional byte
+        if (pstate->unreadLen)

Review Comment:
   I've renamed `unreadLen` to `numUnreadBits`/`numUnwritBits`, which better matches a local variable called `num_bits` in the C code using them.



##########
daffodil-codegen-c/src/main/scala/org/apache/daffodil/codegen/c/generators/CodeGeneratorState.scala:
##########
@@ -400,7 +427,7 @@ class CodeGeneratorState(private val root: ElementBase) {
       if (numChildren > 0)
         s"""static const $C ${C}_compute_offsets;
          |
-         |static const size_t ${C}_offsets[$count] = {
+         |static const size_t ${C}_childrenOffsets[$count] = {

Review Comment:
   Yes, I didn't want to do a comprehensive job of renaming in this PR.  The remaining camel case identifiers are in "types", which include struct field names as well, and it sort of makes sense to keep them closer to Daffodil's camel case style since both the Daffodil and C types talk about similar parsing/unparsing/metadata data.



##########
daffodil-codegen-c/src/test/examples/NestedUnion/generated_code.c:
##########
@@ -94,7 +94,7 @@ static const ERD foo_data_NestedUnionType_ERD = {
     },
     COMPLEX, // typeCode
     3, // numChildren
-    foo_data_NestedUnionType__offsets, // offsets
+    foo_data_NestedUnionType__childrenOffsets, // childrenOffsets

Review Comment:
   Good point.  I've removed redundant comments where they suffix similar identifiers and left comments where they suffix NULLs.



##########
daffodil-codegen-c/src/main/resources/org/apache/daffodil/codegen/c/files/libruntime/unparsers.c:
##########
@@ -465,3 +465,30 @@ unparse_check_bounds(const char *name, size_t count, size_t minOccurs, size_t ma
         ustate->error = &error;
     }
 }
+
+// Flush any fractional bits not written yet
+
+void
+flush_ustate(UState *ustate)

Review Comment:
   Yes, I was collapsing "fractional byte's bits" to two words.  I've changed "fractional bits" back to "fractional byte" in comments and renamed `flush_ustate` to `flush_fractional_byte`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org