stevedlawrence commented on code in PR #1248:
URL: https://github.com/apache/daffodil/pull/1248#discussion_r1619788199
##########
daffodil-runtime1-layers/src/main/resources/org/apache/daffodil/layers/xsd/byteSwapLayer.dfdl.xsd:
##########
@@ -26,12 +26,10 @@
<annotation>
<appinfo source="http://www.ogf.org/dfdl/";>
- <!--
- The twobyteswap and fourbyteswap layers have no parameters nor
- return variables
- -->
+ <!-- set/bind to yes if you want a parse error when the length is not a
multiple of the word size -->
+ <dfdl:defineVariable name="requireLengthInWholeWords" type="xs:string"
defaultValue="no"/>
Review Comment:
This is a good example of why it's useful to have a convention where each
layer has a .dfdl.xsd file and schema import these files, even if they are
empty. Not only does it provide a place to add variables if the layer changes,
but it also allows schema compatibility with new versions of Daffodil. Say a
schema was written with the old layer and it imports the empty file. If it
later upgrade to a new version of Daffodil then it will import the same file
but with the new defineVariable and default value, so no changes to the schema
a required to use the updated layer.
##########
daffodil-runtime1-layers/src/main/scala/org/apache/daffodil/layers/runtime1/ByteSwapLayer.scala:
##########
@@ -137,14 +180,15 @@ class ByteSwapInputStream(wordsize: Int, jis:
InputStream) extends InputStream {
* order 4 3 2 1 8 7 6 5 10 9. If wordsize were 2 then the bytes are written
* to the wrapped output stream in the order 2 1 4 3 6 5 8 7 10 9.
*/
-class ByteSwapOutputStream(wordsize: Int, jos: OutputStream) extends
OutputStream {
+class ByteSwapOutputStream(layer: Layer, wordsize: Int, jos: OutputStream)
+ extends OutputStream {
- private val stack: Deque[Byte] = new ArrayDeque[Byte](wordsize)
+ private val stack: Deque[Byte] = new util.ArrayDeque[Byte](wordsize)
private var closed = false
override def close(): Unit = {
if (!closed) {
- while (!stack.isEmpty()) {
+ while (!stack.isEmpty) {
Review Comment:
If `requireWholeWords` is true when closing, should this error if the stack
is not empty? I think a non-empty stack here implies the underlying output
stream didn't write a multiple of wordsize bytes and so we are missing a whole
word?
##########
daffodil-runtime1-layers/src/main/scala/org/apache/daffodil/layers/runtime1/ByteSwapLayer.scala:
##########
@@ -19,6 +19,7 @@ package org.apache.daffodil.layers.runtime1
import java.io.InputStream
import java.io.OutputStream
+import java.util
Review Comment:
Can we remove this import? This is only used once for `new util.ArrayDeque`
and we already import that below.
##########
daffodil-runtime1-layers/src/main/scala/org/apache/daffodil/layers/runtime1/ByteSwapLayer.scala:
##########
@@ -29,18 +30,47 @@ final class TwoByteSwapLayer extends
ByteSwap("twobyteswap", 2)
final class FourByteSwapLayer extends ByteSwap("fourbyteswap", 4)
-abstract class ByteSwap(name: String, count: Int)
+/**
+ * This class represents a byte swap layer that can be used to swap the byte
order of data.
+ *
+ * DFDL Variable `requireLengthInWholeWords` can be used to request that the
layer enforce
+ * the length being a multiple of the word size.
+ *
+ * @constructor Creates a new ByteSwap instance with the specified name and
word size.
+ * @param name The name of the byte swap layer.
+ * @param wordsize The word size in bytes.
+ */
+abstract class ByteSwap(name: String, wordsize: Int)
extends Layer(name, "urn:org.apache.daffodil.layers.byteSwap") {
+ private var wholeWords: Boolean = false
+
+ /**
+ * Initialize from DFDL variables that are parameters.
+ * @param requireLengthInWholeWords a string that is the value of the DFDL
variable of the same name in this layer's
+ * namespace. Must be "yes" or "no" or it
is a SDE.
+ */
+ def setLayerVariableParameters(requireLengthInWholeWords: String): Unit = {
+ requireLengthInWholeWords match {
+ case "yes" => this.wholeWords = true
+ case "no" => this.wholeWords = false // this is the default
Review Comment:
Would it be useful to have something other than yes/no? For example, I'm
thinking maybe an additional value could allow non-whole words on parse but pad
to a whole word on unparse. I'm not sure we need that behavior, but making the
variable name something like `nonWholeWordPolicy` (not sure about that name)
could have accept/error instead of yes/no, but could let you extend it in the
future to implement something like "acceptAndPad". Though I guess
`requirLengthInWholeWords="noButPad"` could still work, so I guess as long as
its a string its fine?
--
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]
