Bram Moolenaar wrote:
> Thanks for the patch. I thought of catching the problem at the cause,
> by disallowing the autocommands to do something that will cause trouble.
> However, it's very difficult to catch all situations.
>
> So I think we can do both: disallow autocommands to do things that are
> clearly wrong, and give an error if we notice side effects. That should
> be safe.
>
> Please have a look at the patch below. There might still be other
> autocommands that cause trouble. If you see one, please report.
...snip...
I applied your patch to Vim-7.2.128.
I still observe something wrong after patching (something
which I did not think of testing earlier, so the problem
I see in your patch was also in my patch).
The bug I see only happens when opening a file in Vim
which does not exist but for which there is a swap file.
Steps to reproduce it:
# Open a file foobar (which does not exist) in vim to create
# swap file .foobar.swp
$ rm -f ~/foobar
$ vim ~/foobar
# In another terminal...
$ vim -u NONE
:au! SwapExists * cd /tmp
:e ~/foobar
The following prompt shows up:
Swap file "~/.foobar.swp" already exists!
[O]pen Read-Only, (E)dit anyway, (R)ecover, (Q)uit, (A)bort:
Press e.
Valgrind then outputs the following error
==17742== Invalid read of size 1
==17742== at 0x810B4DA: get_past_head (misc1.c:4472)
==17742== by 0x810B428: gettail_sep (misc1.c:4419)
==17742== by 0x810B610: dir_of_file_exists (misc1.c:4582)
==17742== by 0x80C13CD: readfile (fileio.c:606)
==17742== by 0x8052D8E: open_buffer (buffer.c:130)
==17742== by 0x8097AC3: do_ecmd (ex_cmds.c:3655)
==17742== by 0x80ADE20: do_exedit (ex_docmd.c:7557)
==17742== by 0x80ADA97: ex_edit (ex_docmd.c:7452)
==17742== by 0x80A6517: do_one_cmd (ex_docmd.c:2622)
==17742== by 0x80A3D97: do_cmdline (ex_docmd.c:1096)
==17742== by 0x8129422: nv_colon (normal.c:5218)
==17742== by 0x8122A86: normal_cmd (normal.c:1189)
==17742== by 0x80E5F8D: main_loop (main.c:1180)
==17742== by 0x80E5ADA: main (main.c:939)
==17742== Address 0x4d34be0 is 0 bytes inside a block of size 15 free'd
==17742== at 0x4024E5A: free (vg_replace_malloc.c:323)
==17742== by 0x8113C72: vim_free (misc2.c:1631)
==17742== by 0x80C8E58: shorten_fnames (fileio.c:5743)
==17742== by 0x80AE6AA: ex_cd (ex_docmd.c:7939)
==17742== by 0x80A6517: do_one_cmd (ex_docmd.c:2622)
==17742== by 0x80A3D97: do_cmdline (ex_docmd.c:1096)
==17742== by 0x80CC9A5: apply_autocmds_group (fileio.c:8883)
==17742== by 0x80CC3A8: apply_autocmds (fileio.c:8494)
==17742== by 0x80F9592: do_swapexists (memline.c:3770)
==17742== by 0x80F9EAD: findswapname (memline.c:4130)
==17742== by 0x80F3FF8: ml_open_file (memline.c:553)
==17742== by 0x80F411E: check_need_swap (memline.c:605)
==17742== by 0x80C13BF: readfile (fileio.c:605)
==17742== by 0x8052D8E: open_buffer (buffer.c:130)
==17742== by 0x8097AC3: do_ecmd (ex_cmds.c:3655)
==17742== by 0x80ADE20: do_exedit (ex_docmd.c:7557)
==17742== by 0x80ADA97: ex_edit (ex_docmd.c:7452)
==17742== by 0x80A6517: do_one_cmd (ex_docmd.c:2622)
==17742== by 0x80A3D97: do_cmdline (ex_docmd.c:1096)
==17742== by 0x8129422: nv_colon (normal.c:5218)
==17742== by 0x8122A86: normal_cmd (normal.c:1189)
==17742== by 0x80E5F8D: main_loop (main.c:1180)
==17742== by 0x80E5ADA: main (main.c:939)
(and several other errors follow)
If I enter ":messages" I also do not see the error
message E812 (I would expect to see this message).
I do not see this issue when the file foobar already exists.
This remaining issue happens because call to
check_need_swap() in fileio.c:605 (or line 592 in
pristine vim-7.2.128) may trigger an autocommand
which can also invalidate fname.
Attached patch (on top of your patch) fixes this
remaining issue (and a typo). I will continue to test
with this patch for a while but so far it looks OK.
I can't see any further bugs so far with autocommands.
Regards
Dominique
--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---
*** fileio.c.old_patch 2009-03-01 10:08:48.000000000 +0100
--- fileio.c 2009-03-01 10:15:24.000000000 +0100
***************
*** 298,304 ****
#ifdef FEAT_AUTOCMD
/* Remember the initial values of curbuf, curbuf->b_ffname and
* curbuf->b_fname to detect whether they are altered as a result of
! * executing nasaty autocommands. Also check if "fname" and "sfname"
* point to one of these values. */
buf_T *old_curbuf = curbuf;
char_u *old_b_ffname = curbuf->b_ffname;
--- 298,304 ----
#ifdef FEAT_AUTOCMD
/* Remember the initial values of curbuf, curbuf->b_ffname and
* curbuf->b_fname to detect whether they are altered as a result of
! * executing nasty autocommands. Also check if "fname" and "sfname"
* point to one of these values. */
buf_T *old_curbuf = curbuf;
char_u *old_b_ffname = curbuf->b_ffname;
***************
*** 602,608 ****
--- 602,620 ----
#ifdef FEAT_QUICKFIX
if (!bt_dontwrite(curbuf))
#endif
+ {
check_need_swap(newfile);
+ #ifdef FEAT_AUTOCMD
+ /* check_need_swap may have triggered autocommand SwapExists */
+ if (curbuf != old_curbuf
+ || (using_b_ffname && (old_b_ffname != curbuf->b_ffname))
+ || (using_b_fname && (old_b_fname != curbuf->b_fname)))
+ {
+ EMSG(_("E812: Autocommands changed buffer or buffer name"));
+ return FAIL;
+ }
+ #endif
+ }
if (dir_of_file_exists(fname))
filemess(curbuf, sfname, (char_u *)_("[New File]"), 0);
else
