sandbox might be set on errors for :s commands

[Christian Brabandt]

Bram,
I see a potential problem when running `:s/../\=Function/gn` command.

If the function aborts, the sandbox might still be set, which causes an
unusable Vim. This just happened to me several times with some errors in
Function. However, when trying to reproduce, aborting() does not seem to
return True with a simple error (like undefined variable or so), so I
don't currently know how to create a test for that :(


So here is just the patch:

diff --git a/src/ex_cmds.c b/src/ex_cmds.c
index b99e54bce..932e3d2f0 100644
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -4880,7 +4880,7 @@ do_sub(exarg_T *eap)
pos_T old_cursor = curwin->w_cursor;
int start_nsubs;
#ifdef FEAT_EVAL
- int save_ma = 0;
+ int save_ma = curbuf->b_p_ma;
#endif

cmd = eap->arg;
@@ -5556,7 +5556,6 @@ do_sub(exarg_T *eap)
if (subflags.do_count)
{
/* prevent accidentally changing the buffer by a function */
- save_ma = curbuf->b_p_ma;
curbuf->b_p_ma = FALSE;
sandbox++;
}
@@ -5571,12 +5570,9 @@ do_sub(exarg_T *eap)
#ifdef FEAT_EVAL
// If getting the substitute string caused an error, don't do
// the replacement.
- if (aborting())
- goto skip;
-
// Don't keep flags set by a recursive call.
subflags = subflags_save;
- if (subflags.do_count)
+ if (aborting() || subflags.do_count)
{
curbuf->b_p_ma = save_ma;
if (sandbox > 0)




Mit freundlichen Grüßen
Christian
--
Blitzt und donnert es mit Schauern, kriecht das Vieh ins Bett zum Bauern.

[Bram Moolenaar]

Christian wrote:

> I see a potential problem when running `:s/../\=Function/gn` command.
>
> If the function aborts, the sandbox might still be set, which causes an
> unusable Vim. This just happened to me several times with some errors in
> Function. However, when trying to reproduce, aborting() does not seem to
> return True with a simple error (like undefined variable or so), so I
> don't currently know how to create a test for that :(

I can reproduce this. There actually is a check for this in
Test_nocatch_sub_failure_handling(). But it doesn't trigger the problem
there. Ah, it doesn't use the "n" flag. Then it indeed fails.

Thanks, that should fix it.

--
If your company is not involved in something called "ISO 9000" you probably
have no idea what it is. If your company _is_ involved in ISO 9000 then you
definitely have no idea what it is.
(Scott Adams - The Dilbert principle)

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///