mbeckerle commented on code in PR #1567:
URL: https://github.com/apache/daffodil/pull/1567#discussion_r2392366554


##########
daffodil-core/src/main/scala/org/apache/daffodil/unparsers/runtime1/BinaryNumberUnparsers.scala:
##########
@@ -261,6 +261,14 @@ abstract class BinaryDecimalUnparserBase(
     val isSigned: Boolean = signed == Yes
     val minWidth: Int = if (isSigned) 2 else 1
     checkMinWidth(state, isSigned, nBits, minWidth)
-    dos.putBigInt(asBigInt(value), nBits, isSigned, finfo)
+    val bigInt = asBigInt(value)
+    if (!isSigned && bigInt.signum() == -1) {
+      UnparseError(
+        One(state.schemaFileLocation),
+        One(state.currentLocation),
+        "Binary decimal value is negative, but decimalSigned is no"

Review Comment:
   Same as prior comment. Add "dfdl:" output the negative value also. 



##########
daffodil-core/src/main/scala/org/apache/daffodil/runtime1/processors/PackedBinaryTraits.scala:
##########
@@ -111,6 +113,10 @@ abstract class PackedBinaryDecimalBaseParser(
 
     try {
       val dec = toPrimType(context, dis.getByteArray(nBits, start))
+      if (dec.getBigDecimal.signum() == -1 && decimalSigned == YesNo.No) {
+        PE(start, "Packed binary decimal value is negative, but decimalSigned 
is no.")

Review Comment:
   add "dfdl:" to property name. Might be better to say "but property 
dfdl:decimalSigned is no." 
   Also would be helpful for this message to output the offending negative 
value. 



##########
daffodil-core/src/main/scala/org/apache/daffodil/unparsers/runtime1/BCDUnparsers.scala:
##########
@@ -69,41 +72,59 @@ final class BCDIntegerMinimumLengthUnparser(e: 
ElementRuntimeData)
   }
 }
 
-abstract class BCDDecimalBaseUnparser(e: ElementRuntimeData, 
binaryDecimalVirtualPoint: Int)
-  extends PackedBinaryDecimalBaseUnparser(e, binaryDecimalVirtualPoint) {
-
-  override def fromBigInteger(bigInt: JBigInteger, nBits: Int): Array[Byte] =
+abstract class BCDDecimalBaseUnparser(
+  e: ElementRuntimeData,
+  binaryDecimalVirtualPoint: Int,
+  decimalSigned: YesNo
+) extends PackedBinaryDecimalBaseUnparser(e, binaryDecimalVirtualPoint, 
decimalSigned) {
+
+  override def fromBigInteger(bigInt: JBigInteger, nBits: Int): Array[Byte] = {
+    if (bigInt.signum == -1) {
+      UnparseError(
+        One(e.schemaFileLocation),
+        Nope,
+        "Signed numbers with dfdl:binaryNumberRep 'bcd' are always only 
positive. %s cannot be negative",

Review Comment:
   You should also output the actual negative value here. Like add "... but 
value was %s." and add the value as an arg. 



##########
daffodil-core/src/main/scala/org/apache/daffodil/unparsers/runtime1/PackedBinaryUnparserTraits.scala:
##########
@@ -100,6 +102,14 @@ abstract class PackedBinaryDecimalBaseUnparser(
         binaryDecimalVirtualPoint
       )
     }
+    if (bigDec.signum() == -1 && decimalSigned == YesNo.No) {
+      UnparseError(
+        One(e.schemaFileLocation),
+        Nope,
+        "Packed binary decimal value is negative, but decimalSigned is no",

Review Comment:
   Same again. 



-- 
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]

Reply via email to