# HG changeset patch # User Bram Moolenaar # Date 1556483406 -7200 # Node ID 4e9bea9b8025d9adaecd9f05956f385c6394f08d # Parent a311ea84a94c661332e4f0b4a7f7d5336063c379 patch 8.1.1231: asking about existing swap file unnecessarily commit https://github.com/vim/vim/commit/67cf86bfff5fd5224d557d81cb146f46e33b831c Author: Bram Moolenaar Date: Sun Apr 28 22:25:38 2019 +0200 patch 8.1.1231: asking about existing swap file unnecessarily Problem: Asking about existing swap file unnecessarily. Solution: When it is safe, delete the swap file. Remove HAS_SWAP_EXISTS_ACTION, it is always defined. (closes #1237) diff --git a/runtime/doc/usr_11.txt b/runtime/doc/usr_11.txt --- a/runtime/doc/usr_11.txt +++ b/runtime/doc/usr_11.txt @@ -1,4 +1,4 @@ -*usr_11.txt* For Vim version 8.1. Last change: 2019 Feb 04 +*usr_11.txt* For Vim version 8.1. Last change: 2019 Apr 28 VIM USER MANUAL - by Bram Moolenaar @@ -205,6 +205,13 @@ 2. The swap file might be the result fro NEWER than swap file! ~ +NOTE that in the following situation Vim knows the swap file is not useful and +will automatically delete it: +- The file is a valid swap file (Magic number is correct). +- The flag that the file was modified is not set. +- The process is not running. + + UNREADABLE SWAP FILE Sometimes the line diff --git a/src/buffer.c b/src/buffer.c --- a/src/buffer.c +++ b/src/buffer.c @@ -972,43 +972,38 @@ goto_buffer( int dir, int count) { -#if defined(HAS_SWAP_EXISTS_ACTION) bufref_T old_curbuf; set_bufref(&old_curbuf, curbuf); swap_exists_action = SEA_DIALOG; -#endif (void)do_buffer(*eap->cmd == 's' ? DOBUF_SPLIT : DOBUF_GOTO, start, dir, count, eap->forceit); -#if defined(HAS_SWAP_EXISTS_ACTION) if (swap_exists_action == SEA_QUIT && *eap->cmd == 's') { -# if defined(FEAT_EVAL) +#if defined(FEAT_EVAL) cleanup_T cs; /* Reset the error/interrupt/exception state here so that * aborting() returns FALSE when closing a window. */ enter_cleanup(&cs); -# endif +#endif /* Quitting means closing the split window, nothing else. */ win_close(curwin, TRUE); swap_exists_action = SEA_NONE; swap_exists_did_quit = TRUE; -# if defined(FEAT_EVAL) +#if defined(FEAT_EVAL) /* Restore the error/interrupt/exception state if not discarded by a * new aborting error, interrupt, or uncaught exception. */ leave_cleanup(&cs); -# endif +#endif } else handle_swap_exists(&old_curbuf); -#endif } -#if defined(HAS_SWAP_EXISTS_ACTION) || defined(PROTO) /* * Handle the situation of swap_exists_action being set. * It is allowed for "old_curbuf" to be NULL or invalid. @@ -1016,21 +1011,21 @@ goto_buffer( void handle_swap_exists(bufref_T *old_curbuf) { -# if defined(FEAT_EVAL) +#if defined(FEAT_EVAL) cleanup_T cs; -# endif -# ifdef FEAT_SYN_HL +#endif +#ifdef FEAT_SYN_HL long old_tw = curbuf->b_p_tw; -# endif +#endif buf_T *buf; if (swap_exists_action == SEA_QUIT) { -# if defined(FEAT_EVAL) +#if defined(FEAT_EVAL) /* Reset the error/interrupt/exception state here so that * aborting() returns FALSE when closing a buffer. */ enter_cleanup(&cs); -# endif +#endif /* User selected Quit at ATTENTION prompt. Go back to previous * buffer. If that buffer is gone or the same as the current one, @@ -1053,26 +1048,26 @@ handle_swap_exists(bufref_T *old_curbuf) // restore msg_silent, so that the command line will be shown msg_silent = old_msg_silent; -# ifdef FEAT_SYN_HL +#ifdef FEAT_SYN_HL if (old_tw != curbuf->b_p_tw) check_colorcolumn(curwin); -# endif +#endif } /* If "old_curbuf" is NULL we are in big trouble here... */ -# if defined(FEAT_EVAL) +#if defined(FEAT_EVAL) /* Restore the error/interrupt/exception state if not discarded by a * new aborting error, interrupt, or uncaught exception. */ leave_cleanup(&cs); -# endif +#endif } else if (swap_exists_action == SEA_RECOVER) { -# if defined(FEAT_EVAL) +#if defined(FEAT_EVAL) /* Reset the error/interrupt/exception state here so that * aborting() returns FALSE when closing a buffer. */ enter_cleanup(&cs); -# endif +#endif /* User selected Recover at ATTENTION prompt. */ msg_scroll = TRUE; @@ -1081,15 +1076,14 @@ handle_swap_exists(bufref_T *old_curbuf) cmdline_row = msg_row; do_modelines(0); -# if defined(FEAT_EVAL) +#if defined(FEAT_EVAL) /* Restore the error/interrupt/exception state if not discarded by a * new aborting error, interrupt, or uncaught exception. */ leave_cleanup(&cs); -# endif +#endif } swap_exists_action = SEA_NONE; } -#endif /* * do_bufdel() - delete or unload buffer(s) @@ -5259,28 +5253,23 @@ ex_buffer_all(exarg_T *eap) continue; /* Open the buffer in this window. */ -#if defined(HAS_SWAP_EXISTS_ACTION) swap_exists_action = SEA_DIALOG; -#endif set_curbuf(buf, DOBUF_GOTO); if (!bufref_valid(&bufref)) { /* autocommands deleted the buffer!!! */ -#if defined(HAS_SWAP_EXISTS_ACTION) swap_exists_action = SEA_NONE; -#endif break; } -#if defined(HAS_SWAP_EXISTS_ACTION) if (swap_exists_action == SEA_QUIT) { -# if defined(FEAT_EVAL) +#if defined(FEAT_EVAL) cleanup_T cs; /* Reset the error/interrupt/exception state here so that * aborting() returns FALSE when closing a window. */ enter_cleanup(&cs); -# endif +#endif /* User selected Quit at ATTENTION prompt; close this window. */ win_close(curwin, TRUE); @@ -5288,16 +5277,15 @@ ex_buffer_all(exarg_T *eap) swap_exists_action = SEA_NONE; swap_exists_did_quit = TRUE; -# if defined(FEAT_EVAL) +#if defined(FEAT_EVAL) /* Restore the error/interrupt/exception state if not * discarded by a new aborting error, interrupt, or uncaught * exception. */ leave_cleanup(&cs); -# endif +#endif } else handle_swap_exists(NULL); -#endif } ui_breakcheck(); diff --git a/src/ex_cmds.c b/src/ex_cmds.c --- a/src/ex_cmds.c +++ b/src/ex_cmds.c @@ -4258,9 +4258,7 @@ do_ecmd( topline = curwin->w_topline; if (!oldbuf) /* need to read the file */ { -#if defined(HAS_SWAP_EXISTS_ACTION) swap_exists_action = SEA_DIALOG; -#endif curbuf->b_flags |= BF_CHECK_RO; /* set/reset 'ro' flag */ /* @@ -4273,11 +4271,9 @@ do_ecmd( (void)open_buffer(FALSE, eap, readfile_flags); #endif -#if defined(HAS_SWAP_EXISTS_ACTION) if (swap_exists_action == SEA_QUIT) retval = FAIL; handle_swap_exists(&old_curbuf); -#endif } else { diff --git a/src/fileio.c b/src/fileio.c --- a/src/fileio.c +++ b/src/fileio.c @@ -684,15 +684,13 @@ readfile( #endif } -#if defined(HAS_SWAP_EXISTS_ACTION) - /* If "Quit" selected at ATTENTION dialog, don't load the file */ + // If "Quit" selected at ATTENTION dialog, don't load the file if (swap_exists_action == SEA_QUIT) { if (!read_buffer && !read_stdin) close(fd); return FAIL; } -#endif ++no_wait_return; /* don't wait for return yet */ diff --git a/src/globals.h b/src/globals.h --- a/src/globals.h +++ b/src/globals.h @@ -966,7 +966,6 @@ EXTERN int emsg_silent INIT(= 0); /* don EXTERN int emsg_noredir INIT(= 0); /* don't redirect error messages */ EXTERN int cmd_silent INIT(= FALSE); /* don't echo the command line */ -# define HAS_SWAP_EXISTS_ACTION EXTERN int swap_exists_action INIT(= SEA_NONE); /* For dialog when swap file already * exists. */ @@ -1644,6 +1643,9 @@ EXTERN int *eval_lavars_used INIT(= NULL #endif #ifdef MSWIN +# ifdef PROTO +typedef int HINSTANCE; +# endif EXTERN int ctrl_break_was_pressed INIT(= FALSE); EXTERN HINSTANCE g_hinst INIT(= NULL); #endif diff --git a/src/main.c b/src/main.c --- a/src/main.c +++ b/src/main.c @@ -50,9 +50,7 @@ static void exe_pre_commands(mparm_T *pa static void exe_commands(mparm_T *parmp); static void source_startup_scripts(mparm_T *parmp); static void main_start_gui(void); -# if defined(HAS_SWAP_EXISTS_ACTION) static void check_swap_exists_action(void); -# endif # ifdef FEAT_EVAL static void set_progpath(char_u *argv0); # endif @@ -784,19 +782,15 @@ vim_main2(void) */ if (params.tagname != NULL) { -#if defined(HAS_SWAP_EXISTS_ACTION) swap_exists_did_quit = FALSE; -#endif vim_snprintf((char *)IObuff, IOSIZE, "ta %s", params.tagname); do_cmdline_cmd(IObuff); TIME_MSG("jumping to tag"); -#if defined(HAS_SWAP_EXISTS_ACTION) /* If the user doesn't want to edit the file then we quit here. */ if (swap_exists_did_quit) getout(1); -#endif } /* Execute any "+", "-c" and "-S" arguments. */ @@ -2625,20 +2619,18 @@ read_stdin(void) { int i; -#if defined(HAS_SWAP_EXISTS_ACTION) - /* When getting the ATTENTION prompt here, use a dialog */ + // When getting the ATTENTION prompt here, use a dialog swap_exists_action = SEA_DIALOG; -#endif + no_wait_return = TRUE; i = msg_didany; set_buflisted(TRUE); - (void)open_buffer(TRUE, NULL, 0); /* create memfile and read file */ + (void)open_buffer(TRUE, NULL, 0); // create memfile and read file no_wait_return = FALSE; msg_didany = i; TIME_MSG("reading stdin"); -#if defined(HAS_SWAP_EXISTS_ACTION) + check_swap_exists_action(); -#endif #if !(defined(AMIGA) || defined(MACOS_X)) /* * Close stdin and dup it from stderr. Required for GPM to work @@ -2741,16 +2733,14 @@ create_windows(mparm_T *parmp UNUSED) if (p_fdls >= 0) curwin->w_p_fdl = p_fdls; #endif -#if defined(HAS_SWAP_EXISTS_ACTION) - /* When getting the ATTENTION prompt here, use a dialog */ + // When getting the ATTENTION prompt here, use a dialog swap_exists_action = SEA_DIALOG; -#endif + set_buflisted(TRUE); /* create memfile, read file */ (void)open_buffer(FALSE, NULL, 0); -#if defined(HAS_SWAP_EXISTS_ACTION) if (swap_exists_action == SEA_QUIT) { if (got_int || only_one_window()) @@ -2768,7 +2758,6 @@ create_windows(mparm_T *parmp UNUSED) } else handle_swap_exists(NULL); -#endif dorewind = TRUE; /* start again */ } ui_breakcheck(); @@ -2865,13 +2854,10 @@ edit_buffers( curwin->w_arg_idx = arg_idx; /* Edit file from arg list, if there is one. When "Quit" selected * at the ATTENTION prompt close the window. */ -# ifdef HAS_SWAP_EXISTS_ACTION swap_exists_did_quit = FALSE; -# endif (void)do_ecmd(0, arg_idx < GARGCOUNT ? alist_name(&GARGLIST[arg_idx]) : NULL, NULL, NULL, ECMD_LASTL, ECMD_HIDE, curwin); -# ifdef HAS_SWAP_EXISTS_ACTION if (swap_exists_did_quit) { /* abort or quit selected */ @@ -2884,7 +2870,6 @@ edit_buffers( win_close(curwin, TRUE); advance = FALSE; } -# endif if (arg_idx == GARGCOUNT - 1) arg_had_last = TRUE; ++arg_idx; @@ -3485,7 +3470,6 @@ usage(void) mch_exit(0); } -#if defined(HAS_SWAP_EXISTS_ACTION) /* * Check the result of the ATTENTION dialog: * When "Quit" selected, exit Vim. @@ -3498,7 +3482,6 @@ check_swap_exists_action(void) getout(1); handle_swap_exists(NULL); } -#endif #endif /* NO_VIM_MAIN */ diff --git a/src/memline.c b/src/memline.c --- a/src/memline.c +++ b/src/memline.c @@ -2159,9 +2159,8 @@ swapfile_info(char_u *fname) { msg_puts(_("\n process ID: ")); msg_outnum(char_to_long(b0.b0_pid)); -#if defined(UNIX) - /* EMX kill() not working correctly, it seems */ - if (kill((pid_t)char_to_long(b0.b0_pid), 0) == 0) +#if defined(UNIX) || defined(MSWIN) + if (mch_process_running((pid_t)char_to_long(b0.b0_pid))) { msg_puts(_(" (STILL RUNNING)")); # if defined(FEAT_GUI_DIALOG) || defined(FEAT_CON_DIALOG) @@ -2193,6 +2192,57 @@ swapfile_info(char_u *fname) return x; } +/* + * Return TRUE if the swap file looks OK and there are no changes, thus it can + * be safely deleted. + */ + static time_t +swapfile_unchanged(char_u *fname) +{ + stat_T st; + int fd; + struct block0 b0; + int ret = TRUE; +#ifdef UNIX + long pid; +#endif + + // must be able to stat the swap file + if (mch_stat((char *)fname, &st) == -1) + return FALSE; + + // must be able to read the first block + fd = mch_open((char *)fname, O_RDONLY | O_EXTRA, 0); + if (fd < 0) + return FALSE; + if (read_eintr(fd, &b0, sizeof(b0)) != sizeof(b0)) + { + close(fd); + return FALSE; + } + + // the ID and magic number must be correct + if (ml_check_b0_id(&b0) == FAIL|| b0_magic_wrong(&b0)) + ret = FALSE; + + // must be unchanged + if (b0.b0_dirty) + ret = FALSE; + +#if defined(UNIX) || defined(MSWIN) + // process must known and not be running + pid = char_to_long(b0.b0_pid); + if (pid == 0L || mch_process_running((pid_t)pid)) + ret = FALSE; +#endif + + // TODO: Should we check if the swap file was created on the current + // system? And the current user? + + close(fd); + return ret; +} + static int recov_file_names(char_u **names, char_u *path, int prepend_dot) { @@ -4757,9 +4807,8 @@ findswapname( if (differ == FALSE && !(curbuf->b_flags & BF_RECOVERED) && vim_strchr(p_shm, SHM_ATTENTION) == NULL) { -#if defined(HAS_SWAP_EXISTS_ACTION) int choice = 0; -#endif + stat_T st; #ifdef CREATE_DUMMY_FILE int did_use_dummy = FALSE; @@ -4779,13 +4828,25 @@ findswapname( #if (defined(UNIX) || defined(VMS)) && (defined(FEAT_GUI_DIALOG) || defined(FEAT_CON_DIALOG)) process_still_running = FALSE; #endif + // It's safe to delete the swap file if all these are true: + // - the edited file exists + // - the swap file has no changes and looks OK + if (mch_stat((char *)buf->b_fname, &st) == 0 + && swapfile_unchanged(fname)) + { + choice = 4; + if (p_verbose > 0) + verb_msg(_("Found a swap file that is not useful, deleting it")); + } + #if defined(FEAT_EVAL) /* * If there is an SwapExists autocommand and we can handle * the response, trigger it. It may return 0 to ask the * user anyway. */ - if (swap_exists_action != SEA_NONE + if (choice == 0 + && swap_exists_action != SEA_NONE && has_autocmd(EVENT_SWAPEXISTS, buf_fname, buf)) choice = do_swapexists(buf, fname); @@ -4850,7 +4911,6 @@ findswapname( } #endif -#if defined(HAS_SWAP_EXISTS_ACTION) if (choice > 0) { switch (choice) @@ -4880,7 +4940,6 @@ findswapname( break; } else -#endif { msg_puts("\n"); if (msg_silent == 0) diff --git a/src/os_unix.c b/src/os_unix.c --- a/src/os_unix.c +++ b/src/os_unix.c @@ -2393,6 +2393,16 @@ mch_get_pid(void) return (long)getpid(); } +/* + * return TRUE if process "pid" is still running + */ + int +mch_process_running(pid_t pid) +{ + // EMX kill() not working correctly, it seems + return kill(pid, 0) == 0; +} + #if !defined(HAVE_STRERROR) && defined(USE_GETCWD) static char * strerror(int err) diff --git a/src/os_win32.c b/src/os_win32.c --- a/src/os_win32.c +++ b/src/os_win32.c @@ -2903,6 +2903,23 @@ mch_get_pid(void) return (long)GetCurrentProcessId(); } +/* + * return TRUE if process "pid" is still running + */ + int +mch_process_running(pid_t pid) +{ + HANDLE hProcess = OpenProcess(PROCESS_QUERY_INFORMATION, 0, (DWORD)pid); + DWORD status = 0; + int ret = FALSE; + + if (hProcess == NULL) + return FALSE; // might not have access + if (GetExitCodeProcess(hProcess, &status) ) + ret = status == STILL_ACTIVE; + CloseHandle(hProcess); + return ret; +} /* * Get name of current directory into buffer 'buf' of length 'len' bytes. diff --git a/src/proto/os_unix.pro b/src/proto/os_unix.pro --- a/src/proto/os_unix.pro +++ b/src/proto/os_unix.pro @@ -27,6 +27,7 @@ int mch_get_user_name(char_u *s, int len int mch_get_uname(uid_t uid, char_u *s, int len); void mch_get_host_name(char_u *s, int len); long mch_get_pid(void); +int mch_process_running(pid_t pid); int mch_dirname(char_u *buf, int len); int mch_FullName(char_u *fname, char_u *buf, int len, int force); int mch_isFullName(char_u *fname); diff --git a/src/proto/os_win32.pro b/src/proto/os_win32.pro --- a/src/proto/os_win32.pro +++ b/src/proto/os_win32.pro @@ -19,6 +19,7 @@ void fname_case(char_u *name, int len); int mch_get_user_name(char_u *s, int len); void mch_get_host_name(char_u *s, int len); long mch_get_pid(void); +int mch_process_running(pid_t pid); int mch_dirname(char_u *buf, int len); long mch_getperm(char_u *name); int mch_setperm(char_u *name, long perm); diff --git a/src/testdir/test_swap.vim b/src/testdir/test_swap.vim --- a/src/testdir/test_swap.vim +++ b/src/testdir/test_swap.vim @@ -164,3 +164,59 @@ func Test_swapname() call delete('Xtest2') call delete('Xtest3') endfunc + +func Test_swapfile_delete() + autocmd! SwapExists + function s:swap_exists() + let v:swapchoice = s:swap_choice + let s:swapname = v:swapname + let s:filename = expand('') + endfunc + augroup test_swapfile_delete + autocmd! + autocmd SwapExists * call s:swap_exists() + augroup END + + + " Create a valid swapfile by editing a file. + split XswapfileText + call setline(1, ['one', 'two', 'three']) + write " file is written, not modified + " read the swapfile as a Blob + let swapfile_name = swapname('%') + let swapfile_bytes = readfile(swapfile_name, 'B') + + " Close the file and recreate the swap file. + " Now editing the file will run into the process still existing + quit + call writefile(swapfile_bytes, swapfile_name) + let s:swap_choice = 'e' + let s:swapname = '' + split XswapfileText + quit + call assert_equal(swapfile_name, s:swapname) + + " Write the swapfile with a modified PID, now it will be automatically + " deleted. Process one should never be Vim. + let swapfile_bytes[24:27] = 0z01000000 + call writefile(swapfile_bytes, swapfile_name) + let s:swapname = '' + split XswapfileText + quit + call assert_equal('', s:swapname) + + " Now set the modified flag, the swap file will not be deleted + let swapfile_bytes[28 + 80 + 899] = 0x55 + call writefile(swapfile_bytes, swapfile_name) + let s:swapname = '' + split XswapfileText + quit + call assert_equal(swapfile_name, s:swapname) + + call delete('XswapfileText') + call delete(swapfile_name) + augroup test_swapfile_delete + autocmd! + augroup END + augroup! test_swapfile_delete +endfunc diff --git a/src/version.c b/src/version.c --- a/src/version.c +++ b/src/version.c @@ -768,6 +768,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 1231, +/**/ 1230, /**/ 1229,