patch 8.1.0736: code for Blob not sufficiently tested

Problem:    Code for Blob not sufficiently tested.
Solution:   Add more tests.  Fix uncovered crash.  Add test_null_blob().

diff --git a/runtime/doc/eval.txt b/runtime/doc/eval.txt
index b9c9ca42ad0f23f7ecf41953794fb58f73129614..a64f249665f510727dc1793e77ccdd7393ab0d36 100644 (file)
--- a/runtime/doc/eval.txt
+++ b/runtime/doc/eval.txt
@@ -1,4 +1,4 @@
-*eval.txt*     For Vim version 8.1.  Last change: 2019 Jan 11
+*eval.txt*     For Vim version 8.1.  Last change: 2019 Jan 13
 
 
                  VIM REFERENCE MANUAL    by Bram Moolenaar
@@ -2528,6 +2528,7 @@ test_autochdir()          none    enable 'autochdir' during startup
 test_feedinput({string})       none    add key sequence to input buffer
 test_garbagecollect_now()      none    free memory right now for testing
 test_ignore_error({expr})      none    ignore a specific error
+test_null_blob()               Blob    null value for testing
 test_null_channel()            Channel null value for testing
 test_null_dict()               Dict    null value for testing
 test_null_job()                        Job     null value for testing
@@ -3129,7 +3130,7 @@ ch_evalraw({handle}, {string} [, {options}])              *ch_evalraw()*
                is removed.
                Note that Vim does not know when the text received on a raw
                channel is complete, it may only return the first part and you
-               need to use ch_readraw() to fetch the rest.
+               need to use |ch_readraw()| to fetch the rest.
                See |channel-use|.
 
                {only available when compiled with the |+channel| feature}
@@ -9338,25 +9339,28 @@ test_ignore_error({expr})                        *test_ignore_error()*
                When the {expr} is the string "RESET" then the list of ignored
                errors is made empty.
 
+test_null_blob()                                       *test_null_blob()*
+               Return a |Blob| that is null. Only useful for testing.
+
 test_null_channel()                                    *test_null_channel()*
-               Return a Channel that is null. Only useful for testing.
+               Return a |Channel| that is null. Only useful for testing.
                {only available when compiled with the +channel feature}
 
 test_null_dict()                                       *test_null_dict()*
-               Return a Dict that is null. Only useful for testing.
+               Return a |Dict| that is null. Only useful for testing.
 
 test_null_job()                                                *test_null_job()*
-               Return a Job that is null. Only useful for testing.
+               Return a |Job| that is null. Only useful for testing.
                {only available when compiled with the +job feature}
 
 test_null_list()                                       *test_null_list()*
-               Return a List that is null. Only useful for testing.
+               Return a |List| that is null. Only useful for testing.
 
 test_null_partial()                                    *test_null_partial()*
-               Return a Partial that is null. Only useful for testing.
+               Return a |Partial| that is null. Only useful for testing.
 
 test_null_string()                                     *test_null_string()*
-               Return a String that is null. Only useful for testing.
+               Return a |String| that is null. Only useful for testing.
 
 test_option_not_set({name})                            *test_option_not_set()*
                Reset the flag that indicates option {name} was set.  Thus it

diff --git a/src/blob.c b/src/blob.c
index fc1d3f6248db144bd35a45422c9a310d6f1c3b13..c260a435721dbaaa43d84d70710f875225609c91 100644 (file)
--- a/src/blob.c
+++ b/src/blob.c
@@ -114,13 +114,16 @@ blob_equal(
     blob_T     *b1,
     blob_T     *b2)
 {
-    int i;
+    int            i;
+    int            len1 = blob_len(b1);
+    int            len2 = blob_len(b2);
 
-    if (b1 == NULL || b2 == NULL)
-       return FALSE;
+    // empty and NULL are considered the same
+    if (len1 == 0 && len2 == 0)
+       return TRUE;
     if (b1 == b2)
        return TRUE;
-    if (blob_len(b1) != blob_len(b2))
+    if (len1 != len2)
        return FALSE;
 
     for (i = 0; i < b1->bv_ga.ga_len; i++)

diff --git a/src/eval.c b/src/eval.c
index 59bddb891ad9c9f17c9193d5e7140b7ed019cc31..78e4eae1526c56ac35e5617b402db5534e49cdcd 100644 (file)
--- a/src/eval.c
+++ b/src/eval.c
@@ -1983,9 +1983,9 @@ get_lval(
                }
                if (rettv != NULL
                        && !(rettv->v_type == VAR_LIST
-                           || rettv->vval.v_list != NULL)
+                                                && rettv->vval.v_list != NULL)
                        && !(rettv->v_type == VAR_BLOB
-                           || rettv->vval.v_blob != NULL))
+                                               && rettv->vval.v_blob != NULL))
                {
                    if (!quiet)
                        EMSG(_("E709: [:] requires a List or Blob value"));
@@ -2109,6 +2109,8 @@ get_lval(
        }
        else if (lp->ll_tv->v_type == VAR_BLOB)
        {
+           long bloblen = blob_len(lp->ll_tv->vval.v_blob);
+
            /*
             * Get the number and item for the only or first index of the List.
             */
@@ -2120,16 +2122,26 @@ get_lval(
            clear_tv(&var1);
 
            if (lp->ll_n1 < 0
-                   || lp->ll_n1 > blob_len(lp->ll_tv->vval.v_blob))
+                   || lp->ll_n1 > bloblen
+                   || (lp->ll_range && lp->ll_n1 == bloblen))
            {
                if (!quiet)
-                   EMSGN(_(e_listidx), lp->ll_n1);
+                   EMSGN(_(e_blobidx), lp->ll_n1);
+               clear_tv(&var2);
                return NULL;
            }
            if (lp->ll_range && !lp->ll_empty2)
            {
                lp->ll_n2 = (long)tv_get_number(&var2);
                clear_tv(&var2);
+               if (lp->ll_n2 < 0
+                       || lp->ll_n2 >= bloblen
+                       || lp->ll_n2 < lp->ll_n1)
+               {
+                   if (!quiet)
+                       EMSGN(_(e_blobidx), lp->ll_n2);
+                   return NULL;
+               }
            }
            lp->ll_blob = lp->ll_tv->vval.v_blob;
            lp->ll_tv = NULL;
@@ -2241,6 +2253,7 @@ set_var_lval(
        if (lp->ll_blob != NULL)
        {
            int     error = FALSE, val;
+
            if (op != NULL && *op != '=')
            {
                EMSG2(_(e_letwrong), op);
@@ -2249,17 +2262,23 @@ set_var_lval(
 
            if (lp->ll_range && rettv->v_type == VAR_BLOB)
            {
-               int     i;
+               int     il, ir;
 
-               if (blob_len(rettv->vval.v_blob) != blob_len(lp->ll_blob))
+               if (lp->ll_empty2)
+                   lp->ll_n2 = blob_len(lp->ll_blob) - 1;
+
+               if (lp->ll_n2 - lp->ll_n1 + 1 != blob_len(rettv->vval.v_blob))
                {
-                   EMSG(_("E972: Blob value has more items than target"));
+                   EMSG(_("E972: Blob value does not have the right number of bytes"));
                    return;
                }
+               if (lp->ll_empty2)
+                   lp->ll_n2 = blob_len(lp->ll_blob);
 
-               for (i = lp->ll_n1; i <= lp->ll_n2; i++)
-                   blob_set(lp->ll_blob, i,
-                           blob_get(rettv->vval.v_blob, i));
+               ir = 0;
+               for (il = lp->ll_n1; il <= lp->ll_n2; il++)
+                   blob_set(lp->ll_blob, il,
+                           blob_get(rettv->vval.v_blob, ir++));
            }
            else
            {
@@ -2278,8 +2297,7 @@ set_var_lval(
                        if (lp->ll_n1 == gap->ga_len)
                            ++gap->ga_len;
                    }
-                   else
-                       EMSG(_(e_invrange));
+                   // error for invalid range was already given in get_lval()
                }
            }
        }
@@ -2312,7 +2330,7 @@ set_var_lval(
     else if (lp->ll_range)
     {
        listitem_T *ll_li = lp->ll_li;
-       int ll_n1 = lp->ll_n1;
+       int         ll_n1 = lp->ll_n1;
 
        /*
         * Check whether any of the list items is locked
@@ -3354,6 +3372,8 @@ eval0(
 {
     int                ret;
     char_u     *p;
+    int                did_emsg_before = did_emsg;
+    int                called_emsg_before = called_emsg;
 
     p = skipwhite(arg);
     ret = eval1(&p, rettv, evaluate);
@@ -3364,9 +3384,11 @@ eval0(
        /*
         * Report the invalid expression unless the expression evaluation has
         * been cancelled due to an aborting error, an interrupt, or an
-        * exception.
+        * exception, or we already gave a more specific error.
+        * Also check called_emsg for when using assert_fails().
         */
-       if (!aborting())
+       if (!aborting() && did_emsg == did_emsg_before
+                                         && called_emsg == called_emsg_before)
            EMSG2(_(e_invexpr2), arg);
        ret = FAIL;
     }
@@ -4195,7 +4217,7 @@ eval7(
                    {
                        if (!vim_isxdigit(bp[1]))
                        {
-                           EMSG(_("E973: Blob literal should have an even number of hex characters'"));
+                           EMSG(_("E973: Blob literal should have an even number of hex characters"));
                            vim_free(blob);
                            ret = FAIL;
                            break;
@@ -4632,7 +4654,7 @@ eval_index(
                len = blob_len(rettv->vval.v_blob);
                if (range)
                {
-                   // The resulting variable is a substring.  If the indexes
+                   // The resulting variable is a sub-blob.  If the indexes
                    // are out of range the result is empty.
                    if (n1 < 0)
                    {
@@ -8336,6 +8358,7 @@ ex_echo(exarg_T *eap)
     int                atstart = TRUE;
     char_u     numbuf[NUMBUFLEN];
     int                did_emsg_before = did_emsg;
+    int                called_emsg_before = called_emsg;
 
     if (eap->skip)
        ++emsg_skip;
@@ -8353,7 +8376,8 @@ ex_echo(exarg_T *eap)
             * has been cancelled due to an aborting error, an interrupt, or an
             * exception.
             */
-           if (!aborting() && did_emsg == did_emsg_before)
+           if (!aborting() && did_emsg == did_emsg_before
+                                         && called_emsg == called_emsg_before)
                EMSG2(_(e_invexpr2), p);
            need_clr_eos = FALSE;
            break;

diff --git a/src/evalfunc.c b/src/evalfunc.c
index a5bae56b89ff8c89925226d510d5db928c784d83..c1c8c23bd5441685a90fc66a8d819775ee6f27d3 100644 (file)
--- a/src/evalfunc.c
+++ b/src/evalfunc.c
@@ -429,6 +429,7 @@ static void f_test_option_not_set(typval_T *argvars, typval_T *rettv);
 static void f_test_override(typval_T *argvars, typval_T *rettv);
 static void f_test_garbagecollect_now(typval_T *argvars, typval_T *rettv);
 static void f_test_ignore_error(typval_T *argvars, typval_T *rettv);
+static void f_test_null_blob(typval_T *argvars, typval_T *rettv);
 #ifdef FEAT_JOB_CHANNEL
 static void f_test_null_channel(typval_T *argvars, typval_T *rettv);
 #endif
@@ -950,6 +951,7 @@ static struct fst
     {"test_feedinput", 1, 1, f_test_feedinput},
     {"test_garbagecollect_now",        0, 0, f_test_garbagecollect_now},
     {"test_ignore_error",      1, 1, f_test_ignore_error},
+    {"test_null_blob", 0, 0, f_test_null_blob},
 #ifdef FEAT_JOB_CHANNEL
     {"test_null_channel", 0, 0, f_test_null_channel},
 #endif
@@ -13902,6 +13904,13 @@ f_test_ignore_error(typval_T *argvars, typval_T *rettv UNUSED)
      ignore_error_for_testing(tv_get_string(&argvars[0]));
 }
 
+    static void
+f_test_null_blob(typval_T *argvars UNUSED, typval_T *rettv)
+{
+    rettv->v_type = VAR_BLOB;
+    rettv->vval.v_blob = NULL;
+}
+
 #ifdef FEAT_JOB_CHANNEL
     static void
 f_test_null_channel(typval_T *argvars UNUSED, typval_T *rettv)

diff --git a/src/testdir/test49.vim b/src/testdir/test49.vim
index 0f825a255b52fcb90e86a2a7c220274698cb9cbc..97088f01e80762d33319b92ac96f2b080c57fc53 100644 (file)
--- a/src/testdir/test49.vim
+++ b/src/testdir/test49.vim
@@ -1,6 +1,6 @@
 " Vim script language tests
 " Author:      Servatius Brandt <Servatius.Brandt@fujitsu-siemens.com>
-" Last Change: 2019 Jan 09
+" Last Change: 2019 Jan 13
 
 "-------------------------------------------------------------------------------
 " Test environment                                                         {{{1
@@ -3694,7 +3694,7 @@ endif
 if ExtraVim(msgfile)
     try
        Xpath 4194304                           " X: 4194304
-       let x = novar   " error E121/E15; exception: E121
+       let x = novar   " error E121; exception: E121
     catch /E15:/       " should not catch
        Xpath 8388608                           " X: 0
     endtry
@@ -3702,7 +3702,7 @@ if ExtraVim(msgfile)
 endif
 
 Xpath 33554432                                 " X: 33554432
-if !MESSAGES('E121', "Undefined variable", 'E15', "Invalid expression")
+if !MESSAGES('E121', "Undefined variable")
     Xpath 67108864                             " X: 0
 endif
 

diff --git a/src/testdir/test_assign.vim b/src/testdir/test_assign.vim
index 317d45867f0626aa6e2112e9ad75f99d9c1fcb73..1715a3f03b3c1d5bb5af20730177a5d1f96d3798 100644 (file)
--- a/src/testdir/test_assign.vim
+++ b/src/testdir/test_assign.vim
@@ -23,11 +23,11 @@ func Test_let_termcap()
     let &t_k1 = old_t_k1
   endif
 
-  call assert_fails('let x = &t_xx', 'E15')
+  call assert_fails('let x = &t_xx', 'E113')
   let &t_xx = "yes"
   call assert_equal("yes", &t_xx)
   let &t_xx = ""
-  call assert_fails('let x = &t_xx', 'E15')
+  call assert_fails('let x = &t_xx', 'E113')
 endfunc
 
 func Test_let_option_error()
@@ -43,3 +43,11 @@ func Test_let_option_error()
   call assert_equal("vert:|", &fillchars)
   let &fillchars = _w
 endfunc
+
+func Test_let_errors()
+  let s = 'abcd'
+  call assert_fails('let s[1] = 5', 'E689:')
+
+  let l = [1, 2, 3]
+  call assert_fails('let l[:] = 5', 'E709:')
+endfunc

diff --git a/src/testdir/test_blob.vim b/src/testdir/test_blob.vim
index 4ab28eb4f1f913a48bff992205f336e620ba7bd0..550b8f1b9f98e1e892843bacff686dcb724fcbbc 100644 (file)
--- a/src/testdir/test_blob.vim
+++ b/src/testdir/test_blob.vim
@@ -21,6 +21,12 @@ func Test_blob_create()
   call assert_equal(0xDE, get(b, 0))
   call assert_equal(0xEF, get(b, 3))
   call assert_fails('let x = get(b, 4)')
+
+  call assert_fails('let b = 0z1', 'E973:')
+  call assert_fails('let b = 0z1x', 'E973:')
+  call assert_fails('let b = 0z12345', 'E973:')
+
+  call assert_equal(0z, test_null_blob())
 endfunc
 
 " assignment to a blob
@@ -32,6 +38,45 @@ func Test_blob_assign()
   let bcopy = b[:]
   call assert_equal(b, bcopy)
   call assert_false(b is bcopy)
+
+  let b = 0zDEADBEEF
+  let b2 = b
+  call assert_true(b is b2)
+  let b[:] = 0z11223344
+  call assert_equal(0z11223344, b)
+  call assert_equal(0z11223344, b2)
+  call assert_true(b is b2)
+
+  let b = 0zDEADBEEF
+  let b[3:] = 0z66
+  call assert_equal(0zDEADBE66, b)
+  let b[:1] = 0z8899
+  call assert_equal(0z8899BE66, b)
+
+  call assert_fails('let b[2:3] = 0z112233', 'E972:')
+  call assert_fails('let b[2:3] = 0z11', 'E972:')
+  call assert_fails('let b[3:2] = 0z', 'E979:')
+
+  let b = 0zDEADBEEF
+  let b += 0z99
+  call assert_equal(0zDEADBEEF99, b)
+
+  call assert_fails('let b .= 0z33', 'E734:')
+  call assert_fails('let b .= "xx"', 'E734:')
+  call assert_fails('let b += "xx"', 'E734:')
+  call assert_fails('let b[1:1] .= 0z55', 'E734:')
+endfunc
+
+func Test_blob_get_range()
+  let b = 0z0011223344
+  call assert_equal(0z2233, b[2:3])
+  call assert_equal(0z223344, b[2:-1])
+  call assert_equal(0z00, b[0:-5])
+  call assert_equal(0z, b[0:-11])
+  call assert_equal(0z44, b[-1:])
+  call assert_equal(0z0011223344, b[:])
+  call assert_equal(0z0011223344, b[:-1])
+  call assert_equal(0z, b[5:6])
 endfunc
 
 func Test_blob_to_string()
@@ -44,8 +89,12 @@ endfunc
 func Test_blob_compare()
   let b1 = 0z0011
   let b2 = 0z1100
+  let b3 = 0z001122
+  call assert_true(b1 == b1)
   call assert_false(b1 == b2)
+  call assert_false(b1 == b3)
   call assert_true(b1 != b2)
+  call assert_true(b1 != b3)
   call assert_true(b1 == 0z0011)
 
   call assert_false(b1 is b2)
@@ -65,7 +114,7 @@ func Test_blob_range_assign()
   let b[1] = 0x11
   let b[2] = 0x22
   call assert_equal(0z001122, b)
-  call assert_fails('let b[4] = 0x33')
+  call assert_fails('let b[4] = 0x33', 'E979:')
 endfunc
 
 func Test_blob_for_loop()
@@ -177,3 +226,15 @@ func Test_blob_json_encode()
   call assert_equal('[222,173,190,239]', json_encode(0zDEADBEEF))
   call assert_equal('[]', json_encode(0z))
 endfunc
+
+func Test_blob_lock()
+  let b = 0z112233
+  lockvar b
+  call assert_fails('let b = 0z44', 'E741:')
+  unlockvar b
+  let b = 0z44
+endfunc
+
+func Test_blob_sort()
+  call assert_fails('call sort([1.0, 0z11], "f")', 'E975:')
+endfunc

diff --git a/src/testdir/test_eval_stuff.vim b/src/testdir/test_eval_stuff.vim
index a1aa7887835c5d8f3c5ee1050a0288c0d4159f7b..dc3cb8919d94590bc1b5ac5e6aeb43f729d7b49e 100644 (file)
--- a/src/testdir/test_eval_stuff.vim
+++ b/src/testdir/test_eval_stuff.vim
@@ -63,3 +63,9 @@ func Test_E963()
   call assert_fails("let v:oldfiles=''", 'E963:')
   call assert_equal(v_o, v:oldfiles)
 endfunc
+
+func Test_for_invalid()
+  call assert_fails("for x in 99", 'E714:')
+  call assert_fails("for x in 'asdf'", 'E714:')
+  call assert_fails("for x in {'a': 9}", 'E714:')
+endfunc

diff --git a/src/testdir/test_lambda.vim b/src/testdir/test_lambda.vim
index 7a31dcc1eff0856db2f39fdcee562444371c0cf2..4ff3e20f1384664be00318dad71c547a92ed4fc5 100644 (file)
--- a/src/testdir/test_lambda.vim
+++ b/src/testdir/test_lambda.vim
@@ -49,7 +49,7 @@ endfunc
 
 function Test_lambda_fails()
   call assert_equal(3, {a, b -> a + b}(1, 2))
-  call assert_fails('echo {a, a -> a + a}(1, 2)', 'E15:')
+  call assert_fails('echo {a, a -> a + a}(1, 2)', 'E853:')
   call assert_fails('echo {a, b -> a + b)}(1, 2)', 'E15:')
 endfunc
 
@@ -169,7 +169,7 @@ func Test_lambda_scope()
   let l:D = s:NewCounter2()
 
   call assert_equal(1, l:C())
-  call assert_fails(':call l:D()', 'E15:') " E121: then E15:
+  call assert_fails(':call l:D()', 'E121:')
   call assert_equal(2, l:C())
 endfunc
 

diff --git a/src/version.c b/src/version.c
index 448e27d87a273e396585aa64d35555612e9aeb3a..643830a6787050f8e2f2cb7b89ea8b334f85afe5 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -795,6 +795,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    736,
 /**/
     735,
 /**/