Berry fix `bytes` setters and getters with negative offsets (#21835)

CHANGELOG.md

@@ -15,6 +15,7 @@ All notable changes to this project will be documented in this file.
 
 ### Fixed
 - Berry `light.get` for separate RGB/CT (#21818)
+- Berry fix `bytes` setters and getters with negative offsets
 
 ### Removed
 

lib/libesp32/berry/src/be_byteslib.c

@@ -868,6 +868,12 @@ static int m_get(bvm *vm, bbool sign)
         if (argc >= 3 && be_isint(vm, 3)) {
             vsize = be_toint(vm, 3);
         }
+        if (idx < 0) {
+            idx = attr.len + idx;       /* if index is negative, count from end */
+        }
+        if (idx < 0) {
+            vsize = 0;                  /* if still negative, then invalid, return 0 */
+        }
         int ret = 0;
         switch (vsize) {
             case 0:     break;
@@ -892,11 +898,7 @@ static int m_get(bvm *vm, bbool sign)
             default:    be_raise(vm, "type_error", "size must be -4, -3, -2, -1, 0, 1, 2, 3 or 4.");
         }
         be_pop(vm, argc - 1);
-        if (vsize != 0) {
         be_pushint(vm, ret);
-        } else {
-            be_pushnil(vm);
-        }
         be_return(vm);
     }
     be_return_nil(vm);
@@ -913,14 +915,20 @@ static int m_getfloat(bvm *vm)
     check_ptr(vm, &attr);
     if (argc >=2 && be_isint(vm, 2)) {
         int32_t idx = be_toint(vm, 2);
+        float ret_f = 0;
+        if (idx < 0) {
+            idx = attr.len + idx;       /* if index is negative, count from end */
+        }
+        if (idx >= 0) {
             bbool be = bfalse;             /* little endian? */
             if (argc >= 3) {
                 be = be_tobool(vm, 3);
             }
             int32_t ret_i = be ? buf_get4_be(&attr, idx) : buf_get4_le(&attr, idx);
-        float* ret_f = (float*) &ret_i;
+            ret_f = *(float*) &ret_i;
+        }
         be_pop(vm, argc - 1);
-        be_pushreal(vm, *ret_f);
+        be_pushreal(vm, ret_f);
         be_return(vm);
     }
     be_return_nil(vm);
@@ -958,6 +966,12 @@ static int m_set(bvm *vm)
         if (argc >= 4 && be_isint(vm, 4)) {
             vsize = be_toint(vm, 4);
         }
+        if (idx < 0) {
+            idx = attr.len + idx;       /* if index is negative, count from end */
+        }
+        if (idx < 0) {
+            vsize = 0;                  /* if still negative, then invalid, do nothing */
+        }
         switch (vsize) {
             case 0:     break;
             case -1:    /* fallback below */
@@ -971,7 +985,7 @@ static int m_set(bvm *vm)
             default:    be_raise(vm, "type_error", "size must be -4, -3, -2, -1, 0, 1, 2, 3 or 4.");
         }
         be_pop(vm, argc - 1);
-        m_write_attributes(vm, 1, &attr);  /* update attributes */
+        // m_write_attributes(vm, 1, &attr);  /* update attributes */
         be_return_nil(vm);
     }
     be_return_nil(vm);
@@ -989,6 +1003,10 @@ static int m_setfloat(bvm *vm)
     check_ptr_modifiable(vm, &attr);
     if (argc >=3 && be_isint(vm, 2) && (be_isint(vm, 3) || be_isreal(vm, 3))) {
         int32_t idx = be_toint(vm, 2);
+        if (idx < 0) {
+            idx = attr.len + idx;       /* if index is negative, count from end */
+        }
+        if (idx >= 0) {
             float val_f = (float) be_toreal(vm, 3);
             int32_t* val_i = (int32_t*) &val_f;
             bbool be = bfalse;
@@ -997,7 +1015,8 @@ static int m_setfloat(bvm *vm)
             }
             if (be) { buf_set4_be(&attr, idx, *val_i); } else { buf_set4_le(&attr, idx, *val_i); }
             be_pop(vm, argc - 1);
-        m_write_attributes(vm, 1, &attr);  /* update attributes */
+            // m_write_attributes(vm, 1, &attr);  /* update attributes */
+        }
         be_return_nil(vm);
     }
     be_return_nil(vm);
@@ -1046,7 +1065,10 @@ static int m_setbytes(bvm *vm)
         int32_t idx = be_toint(vm, 2);
         size_t from_len_total;
         const uint8_t* buf_ptr = (const uint8_t*) be_tobytes(vm, 3, &from_len_total);
-        if (idx < 0) { idx = 0; }
+        if (idx < 0) {
+            idx = attr.len + idx;       /* if index is negative, count from end */
+        }
+        if (idx < 0) { idx = 0; }       /* if still negative, start from offset 0 */
         if (idx >= attr.len) { idx = attr.len; }
 
         int32_t from_byte = 0;
@@ -1093,6 +1115,7 @@ static int m_reverse(bvm *vm)
 
     if (argc >= 2 && be_isint(vm, 2)) {
         idx = be_toint(vm, 2);
+        if (idx < 0) { idx = attr.len + idx; }  /* if negative, count from end */
         if (idx < 0) { idx = 0; }               /* railguards */
         if (idx > attr.len) { idx = attr.len; }
     }
@@ -1140,9 +1163,12 @@ static int m_setitem(bvm *vm)
     if (argc >=3 && be_isint(vm, 2) && be_isint(vm, 3)) {
         int index = be_toint(vm, 2);
         int val = be_toint(vm, 3);
+        if (index < 0) {
+            index += attr.len;       /* if index is negative, count from end */
+        }
         if (index >= 0 && index < attr.len) {
             buf_set1(&attr, index, val);
-            m_write_attributes(vm, 1, &attr);  /* update attributes */
+            // m_write_attributes(vm, 1, &attr);  /* update attributes */
             be_return_nil(vm);
         }
     }

lib/libesp32/berry/tests/bytes.be

@@ -1,3 +1,12 @@
+def assert_error(f, error_type)
+    try
+        f()
+        assert(false, 'unexpected execution flow')
+    except .. as e, m
+        assert(e == error_type)
+    end
+end
+
 #- basic initialization -#
 b=bytes()
 assert(str(b) == "bytes('')")
@@ -48,18 +57,22 @@ assert(str(b) == "bytes('227878567856341278567812345678')")
 #- get -#
 b=bytes("000102030405")
 assert(b.get(0) == 0)
-assert(b.get(-1) == 0)  #- could consider nil as well -#
+assert(b.get(-1) == 0x05)   #- last byte -#
-assert(b.get(6) == 0)  #- could consider nil as well -#
+assert(b.get(6) == 0)
 assert(b.get(1) == 1)
 assert(b.get(5) == 5)
+assert(b.get(-1000) == 0)   # out of range, default to zero
+assert(b.get(1000) == 0)    # out of range, default to zero
 
-assert(b.get(1,0) == nil)
 assert(b.get(1,1) == 0x01)
 assert(b.get(1,2) == 0x0201)
 assert(b.get(1,4) == 0x04030201)
 assert(b.get(1,-1) == 0x01)
 assert(b.get(1,-2) == 0x0102) #- big endian -#
 assert(b.get(1,-4) == 0x01020304)
+assert(b.get(1,0) == 0)     # size zero is invalid, returns zero
+assert(b.get(-1000,1) == 0) # out of range, default to zero
+assert(b.get(1000,1) == 0)  # out of range, default to zero
 
 #- resize -#
 assert(bytes().size() == 0)
@@ -76,6 +89,10 @@ assert(b.size() == 20)
 b.resize(0)
 assert(str(b) == "bytes('')")
 assert(b.size() == 0)
+b=bytes("112233")
+b.resize(-5)    # resize negative is equivalent to resize(0)
+assert(str(b) == "bytes('')")
+assert(b.size() == 0)
 
 #- clear -#
 b=bytes("aabb")
@@ -110,7 +127,7 @@ assert(str(b) == "bytes('AA')")
 b = b1 + '01'
 assert(str(b) == "bytes('AA3031')")
 
-#- .. and append as synonym-#
+#- .. and append as synonyms -#
 b1 = bytes("1122")
 b2 = bytes("334455")
 b = b1..b2
@@ -143,6 +160,12 @@ b = bytes("334455")
 assert(b[0] == 0x33)
 assert(b[1] == 0x44)
 assert(b[2] == 0x55)
+assert(b[-1] == 0x55)
+assert(b[-2] == 0x44)
+assert(b[-3] == 0x33)
+# out of range raises "index_error" exceptions
+assert_error(def () return b[-4] end, 'index_error')
+assert_error(def () return b[4] end, 'index_error')
 
 #- item range -#
 b = bytes("00112233445566778899AABBCCDDEEFF")
@@ -169,6 +192,14 @@ b[0]=0xBB
 assert(str(b) =="bytes('BBAA33')")
 b[2]=-1
 assert(str(b) =="bytes('BBAAFF')")
+# negative indices, counting from end
+b[-1]=0xFE
+assert(str(b) =="bytes('BBAAFE')")
+b[-3]=0xBC
+assert(str(b) =="bytes('BCAAFE')")
+# out of range raises "index_error" exceptions
+assert_error(def () b[-4]=0x11 end, 'index_error')
+assert_error(def () b[4]=0x11 end, 'index_error')
 
 #- resize -#
 b=bytes()
@@ -191,9 +222,7 @@ b.fromstring("Aa0")
 assert(str(b) =="bytes('416130')")
 b=bytes()
 b.fromstring("Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.")
-assert(str(b) =="bytes('4C6F72656D20697073756D20646F6C6F722073697420616D65742C20636F6E73...')")
 assert(b.tostring(0) =="bytes('4C6F72656D20697073756D20646F6C6F722073697420616D65742C20636F6E73656374657475722061646970697363696E6720656C69742C2073656420646F20656975736D6F642074656D706F7220696E6369646964756E74207574206C61626F726520657420646F6C6F7265206D61676E6120616C697175612E')")
-
 assert(size(bytes('4C6F72656D20697073756D20646F6C6F722073697420616D65742C20636F6E73656374657475722061646970697363696E6720656C69742C2073656420646F20656975736D6F642074656D706F7220696E6369646964756E74207574206C61626F726520657420646F6C6F7265206D61676E6120616C697175612E')) == 123)
 
 #- negative index -#
@@ -217,6 +246,13 @@ b.setfloat(0, 0.33)
 assert(b == bytes('C3F5A83E'))
 b = bytes("0000C03F")
 assert(b.getfloat(0) == 1.5)
+b.addfloat(0.33)
+assert(b == bytes("0000C03FC3F5A83E"))
+b.addfloat(0.33, true)      #- Big Endian -#
+assert(b == bytes("0000C03FC3F5A83E3EA8F5C3"))
+b = bytes("")
+b.addfloat(42)      #- add integer -#
+assert(b == bytes("00002842"))
 
 #- fromhex -#
 b = bytes("112233")
@@ -264,7 +300,7 @@ assert(bytes("0011223344").reverse(3) == bytes("0011224433"))
 assert(bytes("0011223344").reverse(4) == bytes("0011223344"))
 assert(bytes("0011223344").reverse(5) == bytes("0011223344"))
 assert(bytes("0011223344").reverse(15) == bytes("0011223344"))
-assert(bytes("0011223344").reverse(-2) == bytes("4433221100"))
+assert(bytes("0011223344").reverse(-2) == bytes("0011224433"))  # reverse starting 2 from end
 
 assert(bytes("0011223344").reverse(1,3) == bytes("0033221144"))
 assert(bytes("0011223344").reverse(1,0) == bytes("0011223344"))