mbeckerle commented on a change in pull request #650:
URL: https://github.com/apache/daffodil/pull/650#discussion_r721479185
##########
File path:
daffodil-runtime2/src/main/resources/org/apache/daffodil/runtime2/c/libruntime/parsers.h
##########
@@ -65,6 +65,14 @@ extern void parse_le_uint8(uint8_t *number, PState *pstate);
extern void parse_fill_bytes(size_t end_position, PState *pstate);
+// Allocate or reallocate memory for hexBinary field
+
+extern void parse_alloc(HexBinary *hexBinary, size_t num_bytes, PState
*pstate);
Review comment:
Why are these two separate calls instead of one that allocates and
populates the allocated box?
##########
File path:
daffodil-runtime2/src/main/resources/org/apache/daffodil/runtime2/c/libruntime/unparsers.c
##########
@@ -133,10 +133,18 @@ unparse_fill_bytes(size_t end_position, const char
fill_byte, UState *ustate)
while (ustate->position < end_position)
{
- write_stream_update_position;
+ write_stream_update_position(buffer.c_val, 1);
}
}
+// Unparse 8-bit bytes from hexBinary field
+
+void
+unparse_hexBinary(HexBinary hexBinary, UState *ustate)
Review comment:
Hmmm. I think this is reading the hexBinary only.
If this was C++ this would be a const * const HexBinary or some such idiom.
In straight C this is pass-by-value.
So this pass by value makes sense to me, but only if we follow this practice
elsewhere and make it a uniform practice.
##########
File path:
daffodil-runtime2/src/main/resources/org/apache/daffodil/runtime2/c/libruntime/infoset.h
##########
@@ -91,6 +94,15 @@ typedef struct ERD
const InitChoiceRD initChoice;
} ERD;
+// HexBinary - parameters of a hexBinary element
+
+typedef struct HexBinary
+{
+ uint8_t *array; // pointer to element's byte array
+ size_t size; // size of element's byte array (in bytes)
Review comment:
DFDL term for this is "length", not size.
A good practice is to use a longer name and give the units e.g.,
lengthInBytes.
It is worthwhile here to anticipate string support a bit. The way strings of
SBCS characters will work is very similar to hexBinary, but ucs-2/utf-16/wchar
strings will have different units of measure perhaps.
##########
File path:
daffodil-runtime2/src/main/resources/org/apache/daffodil/runtime2/c/libcli/xml_reader.c
##########
@@ -217,6 +217,81 @@ strtounum(const char *numptr, uintmax_t maxval, const
Error **errorptr)
return value;
}
+// Store an XML element's string of hexadecimal characters (two
+// nibbles per byte) into a byte array. Allocate memory for byte
+// array if needed. Return error if string does not fit into byte
+// array or does not contain valid hexadecimal characters.
+
+static const Error *
+hexToBinary(const char *hexstring, HexBinary *hexBinary)
Review comment:
I am surprised you are having to write this at all. I expect to see a
call to an XML library for this, with maybe some extra checks around it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
