stevedlawrence commented on code in PR #988:
URL: https://github.com/apache/daffodil/pull/988#discussion_r1139069142
##########
daffodil-io/src/main/scala/org/apache/daffodil/io/InputSourceDataInputStream.scala:
##########
@@ -129,23 +129,14 @@ final class InputSourceDataInputStream private (val
inputSource: InputSource)
def hasReachedEndOfData: Boolean = inputSource.hasReachedEndOfData
/**
- * The number of bits remaining if known. If a limit has not been set,
- * returns Nope. If a limit has been set, the number of bits available
- * until the limit as long as the end of data has not been seen. If the
- * end of data has been seen, the smaller value of the remaining bits
- * in the limit and the remaining bits in the data stream.
+ * Return the number of currently available bytes.
+ *
+ * This should not be used to determine the length of the data, as more bytes
+ * may become available in the future. This should really only be used for
+ * debug purposes.
Review Comment:
Might say "for debug or diagnostic purposes", just to make it clear it is
safe to use in things like PEs.
Might also be worth mention that this does not take into account the
bitLimit just so it's clear.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/Parser.scala:
##########
@@ -65,7 +66,27 @@ sealed trait Parser extends Processor {
pstate.setFailed(new ParseError(One(sfl), One(dataLoc), s, args: _*))
}
- def PENotEnoughBits(pstate: PState, neededBits: Long, remainingBits:
MaybeULong) = {
+ def PENotEnoughBits(pstate: PState, neededBits: Long, source:
InputSourceDataInputStream): Unit = {
+ val remainingLimitedBits = {
+ if (source.bitLimit0b.isEmpty) MaybeULong.Nope
+ else {
+ val lim = source.bitLimit0b.get
+ Assert.invariant(lim >= 0)
+ val nBits = lim - source.bitPos0b
+ MaybeULong(nBits)
+ }
+ }
+
+ val remainingBits = {
+ if (source.hasReachedEndOfData) {
+ val bitsAvailable = source.knownBytesAvailable * 8
+ if (remainingLimitedBits.isDefined && bitsAvailable <
remainingLimitedBits.get)
Review Comment:
Should we also return `bitsAvailable` if there is no bit limit? Maybe
something like this?
```scala
if (remainingLimitedBits.isEmpty || bitsAvailable < remainingLimitsBits.get)
{
MaybeULong(bitsAvailable)
} else {
remainingLimitedBits
}
```
If there's no bitlimit, it must have meant we hit EOF and just didn't have
enough data, so there are bitsAvailable remaining bits.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/Parser.scala:
##########
@@ -65,7 +66,27 @@ sealed trait Parser extends Processor {
pstate.setFailed(new ParseError(One(sfl), One(dataLoc), s, args: _*))
}
- def PENotEnoughBits(pstate: PState, neededBits: Long, remainingBits:
MaybeULong) = {
+ def PENotEnoughBits(pstate: PState, neededBits: Long, source:
InputSourceDataInputStream): Unit = {
+ val remainingLimitedBits = {
+ if (source.bitLimit0b.isEmpty) MaybeULong.Nope
+ else {
+ val lim = source.bitLimit0b.get
+ Assert.invariant(lim >= 0)
+ val nBits = lim - source.bitPos0b
+ MaybeULong(nBits)
+ }
+ }
+
+ val remainingBits = {
+ if (source.hasReachedEndOfData) {
+ val bitsAvailable = source.knownBytesAvailable * 8
Review Comment:
Do we need to take into account fragment bytes?
For example, say there is only 1 byte in the data, we have consumed a single
bit (so `bitPos0b` is 1 and 7 bits are available) and we want to read 8 bits.
I'm not sure if `knownBytesAvailable` returns `0` or `1` in this case (I
*think* 1), but the error would either say "0 bits available" or "8 bits
available" and both are wrong and potentially confusing, since only 7 bits are
available. We may need to look at the `bitPosition % 8` and adjust
`bitsAvailable` if it's non-zero?
Assuming `knowBytesAvailable` returns 1 in this example, I think we just
subtract fragment bits? Something like
```scala
val bitsAvailable = (source.knownBytesAvailable * 8) - (source.bitPos0b % 8)
```
--
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]
