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().

Author: brammool

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

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++)

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;

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)

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,15 +3694,15 @@ 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
     Xpath 16777216				" X: 0
 endif
 
 Xpath 33554432					" X: 33554432
-if !MESSAGES('E121', "Undefined variable", 'E15', "Invalid expression")
+if !MESSAGES('E121', "Undefined variable")
     Xpath 67108864				" X: 0
 endif
 

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