unpack: consistent error messages

between pack and unpack

Signed-off-by: Steve Bennett <steveb@workware.net.au>

2 files changed, 17 insertions, 23 deletions

diff --git a/jim-pack.c b/jim-pack.c
index 61741fc..6518192 100644
--- a/jim-pack.c
+++ b/jim-pack.c
@@ -293,20 +293,24 @@ static int Jim_UnpackCmd(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
     if (Jim_GetWide(interp, argv[3], &pos) != JIM_OK) {
         return JIM_ERR;
     }
+    if (pos < 0 || (option == OPT_STR && pos % 8)) {
+        Jim_SetResultFormatted(interp, "bad bitoffset: %#s", argv[3]);
+        return JIM_ERR;
+    }
     if (Jim_GetWide(interp, argv[4], &width) != JIM_OK) {
         return JIM_ERR;
     }
+    if (width < 0 || (option == OPT_STR && width % 8) || (option != OPT_STR && width > sizeof(jim_wide) * 8) ||
+       ((option == OPT_FLOATLE || option == OPT_FLOATBE) && width != 32 && width != 64)) {
+        Jim_SetResultFormatted(interp, "bad bitwidth: %#s", argv[4]);
+        return JIM_ERR;
+    }
 
     if (option == OPT_STR) {
         int len;
         const char *str = Jim_GetString(argv[1], &len);
 
-        if (width % 8 || pos % 8) {
-            Jim_SetResultString(interp, "string field is not on a byte boundary", -1);
-            return JIM_ERR;
-        }
-
-        if (pos >= 0 && width > 0 && pos < len * 8) {
+        if (pos < len * 8) {
             if (pos + width > len * 8) {
                 width = len * 8 - pos;
             }
@@ -319,12 +323,7 @@ static int Jim_UnpackCmd(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
         const unsigned char *str = (const unsigned char *)Jim_GetString(argv[1], &len);
         jim_wide result = 0;
 
-        if (width > sizeof(jim_wide) * 8) {
-            Jim_SetResultFormatted(interp, "int field is too wide: %#s", argv[4]);
-            return JIM_ERR;
-        }
-
-        if (pos >= 0 && width > 0 && pos < len * 8) {
+        if (pos < len * 8) {
             if (pos + width > len * 8) {
                 width = len * 8 - pos;
             }
@@ -344,11 +343,8 @@ static int Jim_UnpackCmd(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
             double fresult;
             if (width == 32) {
                 fresult = (double) JimIntToFloat(result);
-            } else if (width == 64) {
-                fresult = JimIntToDouble(result);
             } else {
-                Jim_SetResultFormatted(interp, "float field has bad bitwidth: %#s", argv[4]);
-                return JIM_ERR;
+                fresult = JimIntToDouble(result);
             }
             Jim_SetResult(interp, Jim_NewDoubleObj(interp, fresult));
         } else {
diff --git a/tests/pack.test b/tests/pack.test
index a01669a..0ce16cd 100644
--- a/tests/pack.test
+++ b/tests/pack.test
@@ -81,26 +81,24 @@ test unpack-1.3 {unpack bad width} -body {
 } -returnCodes error -result {expected integer but got "badint"}
 
 test unpack-1.4 {unpack bad width} -body {
-	# Poor message
 	unpack abc -intle 0 -5
-} -returnCodes error -result {int field is too wide: -5}
+} -returnCodes error -result {bad bitwidth: -5}
 
 test unpack-1.5 {unpack bad offset} -body {
 	unpack abc -intle badint 8
 } -returnCodes error -result {expected integer but got "badint"}
 
-test unpack-1.6 {unpack bad offset} {
-	# Should be an error
+test unpack-1.6 {unpack bad offset} -body {
 	unpack abc -intle -6 8
-} 0
+} -returnCodes error -result {bad bitoffset: -6}
 
 test unpack-1.7 {unpack str not on byte boundary offset} -body {
 	unpack abc -str 5 8
-} -returnCodes error -result {string field is not on a byte boundary}
+} -returnCodes error -result {bad bitoffset: 5}
 
 test unpack-1.8 {unpack float bad width} -body {
 	unpack abc -floatbe 0 24
-} -returnCodes error -result {float field has bad bitwidth: 24}
+} -returnCodes error -result {bad bitwidth: 24}
 
 test unpack-2.1 {unpack str width past end} -body {
 	unpack abc -str 16 16