Mercurial > vim
changeset 11605:16ccaedce025 v8.0.0685
patch 8.0.0685: when conversion fails written file may be truncated
commit https://github.com/vim/vim/commit/e6bf655bc4de1b7f4586e1f5c2fc4978141c3aa3
Author: Bram Moolenaar <Bram@vim.org>
Date: Tue Jun 27 22:11:51 2017 +0200
patch 8.0.0685: when conversion fails written file may be truncated
Problem: When making backups is disabled and conversion with iconv fails
the written file is truncated. (Luo Chen)
Solution: First try converting the file and write the file only when it did
not fail. (partly by Christian Brabandt)
| author | Christian Brabandt <cb@256bit.org> |
|---|---|
| date | Tue, 27 Jun 2017 22:15:03 +0200 |
| parents | ba60f4a8956b |
| children | 8a62662634fb |
| files | src/fileio.c src/testdir/test_writefile.vim src/version.c |
| diffstat | 3 files changed, 455 insertions(+), 372 deletions(-) [+] |
line wrap: on
line diff
--- a/src/fileio.c +++ b/src/fileio.c @@ -3166,6 +3166,7 @@ buf_write( int device = FALSE; /* writing to a device */ stat_T st_old; int prev_got_int = got_int; + int checking_conversion; int file_readonly = FALSE; /* overwritten file is read-only */ static char *err_readonly = "is read-only (cannot override: \"W\" in 'cpoptions')"; #if defined(UNIX) /*XXX fix me sometime? */ @@ -4344,433 +4345,491 @@ buf_write( #endif /* - * Open the file "wfname" for writing. - * We may try to open the file twice: If we can't write to the - * file and forceit is TRUE we delete the existing file and try to create - * a new one. If this still fails we may have lost the original file! - * (this may happen when the user reached his quotum for number of files). - * Appending will fail if the file does not exist and forceit is FALSE. + * If conversion is taking place, we may first pretend to write and check + * for conversion errors. Then loop again to write for real. + * When not doing conversion this writes for real right away. */ - while ((fd = mch_open((char *)wfname, O_WRONLY | O_EXTRA | (append - ? (forceit ? (O_APPEND | O_CREAT) : O_APPEND) - : (O_CREAT | O_TRUNC)) - , perm < 0 ? 0666 : (perm & 0777))) < 0) + for (checking_conversion = TRUE; ; checking_conversion = FALSE) { /* - * A forced write will try to create a new file if the old one is - * still readonly. This may also happen when the directory is - * read-only. In that case the mch_remove() will fail. + * There is no need to check conversion when: + * - there is no conversion + * - we make a backup file, that can be restored in case of conversion + * failure. */ - if (errmsg == NULL) - { -#ifdef UNIX - stat_T st; - - /* Don't delete the file when it's a hard or symbolic link. */ - if ((!newfile && st_old.st_nlink > 1) - || (mch_lstat((char *)fname, &st) == 0 - && (st.st_dev != st_old.st_dev - || st.st_ino != st_old.st_ino))) - errmsg = (char_u *)_("E166: Can't open linked file for writing"); - else -#endif - { - errmsg = (char_u *)_("E212: Can't open file for writing"); - if (forceit && vim_strchr(p_cpo, CPO_FWRITE) == NULL - && perm >= 0) +#ifdef FEAT_MBYTE + if (!converted || dobackup) +#endif + checking_conversion = FALSE; + + if (checking_conversion) + { + /* Make sure we don't write anything. */ + fd = -1; + write_info.bw_fd = fd; + } + else + { + /* + * Open the file "wfname" for writing. + * We may try to open the file twice: If we can't write to the file + * and forceit is TRUE we delete the existing file and try to + * create a new one. If this still fails we may have lost the + * original file! (this may happen when the user reached his + * quotum for number of files). + * Appending will fail if the file does not exist and forceit is + * FALSE. + */ + while ((fd = mch_open((char *)wfname, O_WRONLY | O_EXTRA | (append + ? (forceit ? (O_APPEND | O_CREAT) : O_APPEND) + : (O_CREAT | O_TRUNC)) + , perm < 0 ? 0666 : (perm & 0777))) < 0) + { + /* + * A forced write will try to create a new file if the old one + * is still readonly. This may also happen when the directory + * is read-only. In that case the mch_remove() will fail. + */ + if (errmsg == NULL) { #ifdef UNIX - /* we write to the file, thus it should be marked - writable after all */ - if (!(perm & 0200)) - made_writable = TRUE; - perm |= 0200; - if (st_old.st_uid != getuid() || st_old.st_gid != getgid()) - perm &= 0777; -#endif - if (!append) /* don't remove when appending */ - mch_remove(wfname); - continue; + stat_T st; + + /* Don't delete the file when it's a hard or symbolic link. + */ + if ((!newfile && st_old.st_nlink > 1) + || (mch_lstat((char *)fname, &st) == 0 + && (st.st_dev != st_old.st_dev + || st.st_ino != st_old.st_ino))) + errmsg = (char_u *)_("E166: Can't open linked file for writing"); + else +#endif + { + errmsg = (char_u *)_("E212: Can't open file for writing"); + if (forceit && vim_strchr(p_cpo, CPO_FWRITE) == NULL + && perm >= 0) + { +#ifdef UNIX + /* we write to the file, thus it should be marked + writable after all */ + if (!(perm & 0200)) + made_writable = TRUE; + perm |= 0200; + if (st_old.st_uid != getuid() + || st_old.st_gid != getgid()) + perm &= 0777; +#endif + if (!append) /* don't remove when appending */ + mch_remove(wfname); + continue; + } + } } - } - } restore_backup: - { - stat_T st; - + { + stat_T st; + + /* + * If we failed to open the file, we don't need a backup. + * Throw it away. If we moved or removed the original file + * try to put the backup in its place. + */ + if (backup != NULL && wfname == fname) + { + if (backup_copy) + { + /* + * There is a small chance that we removed the + * original, try to move the copy in its place. + * This may not work if the vim_rename() fails. + * In that case we leave the copy around. + */ + /* If file does not exist, put the copy in its + * place */ + if (mch_stat((char *)fname, &st) < 0) + vim_rename(backup, fname); + /* if original file does exist throw away the copy + */ + if (mch_stat((char *)fname, &st) >= 0) + mch_remove(backup); + } + else + { + /* try to put the original file back */ + vim_rename(backup, fname); + } + } + + /* if original file no longer exists give an extra warning + */ + if (!newfile && mch_stat((char *)fname, &st) < 0) + end = 0; + } + +#ifdef FEAT_MBYTE + if (wfname != fname) + vim_free(wfname); +#endif + goto fail; + } + write_info.bw_fd = fd; + +#if defined(MACOS_CLASSIC) || defined(WIN3264) + /* TODO: Is it need for MACOS_X? (Dany) */ /* - * If we failed to open the file, we don't need a backup. Throw it - * away. If we moved or removed the original file try to put the - * backup in its place. + * On macintosh copy the original files attributes (i.e. the backup) + * This is done in order to preserve the resource fork and the + * Finder attribute (label, comments, custom icons, file creator) */ - if (backup != NULL && wfname == fname) + if (backup != NULL && overwriting && !append) { if (backup_copy) - { - /* - * There is a small chance that we removed the original, - * try to move the copy in its place. - * This may not work if the vim_rename() fails. - * In that case we leave the copy around. - */ - /* If file does not exist, put the copy in its place */ - if (mch_stat((char *)fname, &st) < 0) - vim_rename(backup, fname); - /* if original file does exist throw away the copy */ - if (mch_stat((char *)fname, &st) >= 0) - mch_remove(backup); - } + (void)mch_copy_file_attribute(wfname, backup); + else + (void)mch_copy_file_attribute(backup, wfname); + } + + if (!overwriting && !append) + { + if (buf->b_ffname != NULL) + (void)mch_copy_file_attribute(buf->b_ffname, wfname); + /* Should copy resource fork */ + } +#endif + +#ifdef FEAT_CRYPT + if (*buf->b_p_key != NUL && !filtering) + { + char_u *header; + int header_len; + + buf->b_cryptstate = crypt_create_for_writing( + crypt_get_method_nr(buf), + buf->b_p_key, &header, &header_len); + if (buf->b_cryptstate == NULL || header == NULL) + end = 0; else { - /* try to put the original file back */ - vim_rename(backup, fname); + /* Write magic number, so that Vim knows how this file is + * encrypted when reading it back. */ + write_info.bw_buf = header; + write_info.bw_len = header_len; + write_info.bw_flags = FIO_NOCONVERT; + if (buf_write_bytes(&write_info) == FAIL) + end = 0; + wb_flags |= FIO_ENCRYPTED; + vim_free(header); } } - - /* if original file no longer exists give an extra warning */ - if (!newfile && mch_stat((char *)fname, &st) < 0) - end = 0; - } +#endif + } + errmsg = NULL; + + write_info.bw_buf = buffer; + nchars = 0; + + /* use "++bin", "++nobin" or 'binary' */ + if (eap != NULL && eap->force_bin != 0) + write_bin = (eap->force_bin == FORCE_BIN); + else + write_bin = buf->b_p_bin; #ifdef FEAT_MBYTE - if (wfname != fname) - vim_free(wfname); -#endif - goto fail; - } - errmsg = NULL; - -#if defined(MACOS_CLASSIC) || defined(WIN3264) - /* TODO: Is it need for MACOS_X? (Dany) */ - /* - * On macintosh copy the original files attributes (i.e. the backup) - * This is done in order to preserve the resource fork and the - * Finder attribute (label, comments, custom icons, file creator) - */ - if (backup != NULL && overwriting && !append) - { - if (backup_copy) - (void)mch_copy_file_attribute(wfname, backup); - else - (void)mch_copy_file_attribute(backup, wfname); - } - - if (!overwriting && !append) - { - if (buf->b_ffname != NULL) - (void)mch_copy_file_attribute(buf->b_ffname, wfname); - /* Should copy resource fork */ - } -#endif - - write_info.bw_fd = fd; - -#ifdef FEAT_CRYPT - if (*buf->b_p_key != NUL && !filtering) - { - char_u *header; - int header_len; - - buf->b_cryptstate = crypt_create_for_writing(crypt_get_method_nr(buf), - buf->b_p_key, &header, &header_len); - if (buf->b_cryptstate == NULL || header == NULL) - end = 0; - else - { - /* Write magic number, so that Vim knows how this file is - * encrypted when reading it back. */ - write_info.bw_buf = header; - write_info.bw_len = header_len; - write_info.bw_flags = FIO_NOCONVERT; - if (buf_write_bytes(&write_info) == FAIL) - end = 0; - wb_flags |= FIO_ENCRYPTED; - vim_free(header); - } - } -#endif - - write_info.bw_buf = buffer; - nchars = 0; - - /* use "++bin", "++nobin" or 'binary' */ - if (eap != NULL && eap->force_bin != 0) - write_bin = (eap->force_bin == FORCE_BIN); - else - write_bin = buf->b_p_bin; - -#ifdef FEAT_MBYTE - /* - * The BOM is written just after the encryption magic number. - * Skip it when appending and the file already existed, the BOM only makes - * sense at the start of the file. - */ - if (buf->b_p_bomb && !write_bin && (!append || perm < 0)) - { - write_info.bw_len = make_bom(buffer, fenc); - if (write_info.bw_len > 0) - { - /* don't convert, do encryption */ - write_info.bw_flags = FIO_NOCONVERT | wb_flags; - if (buf_write_bytes(&write_info) == FAIL) - end = 0; - else - nchars += write_info.bw_len; - } - } - write_info.bw_start_lnum = start; + /* + * The BOM is written just after the encryption magic number. + * Skip it when appending and the file already existed, the BOM only + * makes sense at the start of the file. + */ + if (buf->b_p_bomb && !write_bin && (!append || perm < 0)) + { + write_info.bw_len = make_bom(buffer, fenc); + if (write_info.bw_len > 0) + { + /* don't convert, do encryption */ + write_info.bw_flags = FIO_NOCONVERT | wb_flags; + if (buf_write_bytes(&write_info) == FAIL) + end = 0; + else + nchars += write_info.bw_len; + } + } + write_info.bw_start_lnum = start; #endif #ifdef FEAT_PERSISTENT_UNDO - write_undo_file = (buf->b_p_udf && overwriting && !append - && !filtering && reset_changed); - if (write_undo_file) - /* Prepare for computing the hash value of the text. */ - sha256_start(&sha_ctx); -#endif - - write_info.bw_len = bufsize; + write_undo_file = (buf->b_p_udf + && overwriting + && !append + && !filtering + && reset_changed + && !checking_conversion); + if (write_undo_file) + /* Prepare for computing the hash value of the text. */ + sha256_start(&sha_ctx); +#endif + + write_info.bw_len = bufsize; #ifdef HAS_BW_FLAGS - write_info.bw_flags = wb_flags; -#endif - fileformat = get_fileformat_force(buf, eap); - s = buffer; - len = 0; - for (lnum = start; lnum <= end; ++lnum) - { - /* - * The next while loop is done once for each character written. - * Keep it fast! - */ - ptr = ml_get_buf(buf, lnum, FALSE) - 1; + write_info.bw_flags = wb_flags; +#endif + fileformat = get_fileformat_force(buf, eap); + s = buffer; + len = 0; + for (lnum = start; lnum <= end; ++lnum) + { + /* + * The next while loop is done once for each character written. + * Keep it fast! + */ + ptr = ml_get_buf(buf, lnum, FALSE) - 1; #ifdef FEAT_PERSISTENT_UNDO - if (write_undo_file) - sha256_update(&sha_ctx, ptr + 1, (UINT32_T)(STRLEN(ptr + 1) + 1)); -#endif - while ((c = *++ptr) != NUL) - { - if (c == NL) - *s = NUL; /* replace newlines with NULs */ - else if (c == CAR && fileformat == EOL_MAC) - *s = NL; /* Mac: replace CRs with NLs */ + if (write_undo_file) + sha256_update(&sha_ctx, ptr + 1, + (UINT32_T)(STRLEN(ptr + 1) + 1)); +#endif + while ((c = *++ptr) != NUL) + { + if (c == NL) + *s = NUL; /* replace newlines with NULs */ + else if (c == CAR && fileformat == EOL_MAC) + *s = NL; /* Mac: replace CRs with NLs */ + else + *s = c; + ++s; + if (++len != bufsize) + continue; + if (buf_write_bytes(&write_info) == FAIL) + { + end = 0; /* write error: break loop */ + break; + } + nchars += bufsize; + s = buffer; + len = 0; +#ifdef FEAT_MBYTE + write_info.bw_start_lnum = lnum; +#endif + } + /* write failed or last line has no EOL: stop here */ + if (end == 0 + || (lnum == end + && (write_bin || !buf->b_p_fixeol) + && (lnum == buf->b_no_eol_lnum + || (lnum == buf->b_ml.ml_line_count + && !buf->b_p_eol)))) + { + ++lnum; /* written the line, count it */ + no_eol = TRUE; + break; + } + if (fileformat == EOL_UNIX) + *s++ = NL; else - *s = c; - ++s; - if (++len != bufsize) - continue; - if (buf_write_bytes(&write_info) == FAIL) - { - end = 0; /* write error: break loop */ - break; - } - nchars += bufsize; - s = buffer; - len = 0; -#ifdef FEAT_MBYTE - write_info.bw_start_lnum = lnum; -#endif - } - /* write failed or last line has no EOL: stop here */ - if (end == 0 - || (lnum == end - && (write_bin || !buf->b_p_fixeol) - && (lnum == buf->b_no_eol_lnum - || (lnum == buf->b_ml.ml_line_count && !buf->b_p_eol)))) - { - ++lnum; /* written the line, count it */ - no_eol = TRUE; - break; - } - if (fileformat == EOL_UNIX) - *s++ = NL; - else - { - *s++ = CAR; /* EOL_MAC or EOL_DOS: write CR */ - if (fileformat == EOL_DOS) /* write CR-NL */ - { - if (++len == bufsize) + { + *s++ = CAR; /* EOL_MAC or EOL_DOS: write CR */ + if (fileformat == EOL_DOS) /* write CR-NL */ + { + if (++len == bufsize) + { + if (buf_write_bytes(&write_info) == FAIL) + { + end = 0; /* write error: break loop */ + break; + } + nchars += bufsize; + s = buffer; + len = 0; + } + *s++ = NL; + } + } + if (++len == bufsize && end) + { + if (buf_write_bytes(&write_info) == FAIL) { + end = 0; /* write error: break loop */ + break; + } + nchars += bufsize; + s = buffer; + len = 0; + + ui_breakcheck(); + if (got_int) + { + end = 0; /* Interrupted, break loop */ + break; + } + } +#ifdef VMS + /* + * On VMS there is a problem: newlines get added when writing + * blocks at a time. Fix it by writing a line at a time. + * This is much slower! + * Explanation: VAX/DECC RTL insists that records in some RMS + * structures end with a newline (carriage return) character, and + * if they don't it adds one. + * With other RMS structures it works perfect without this fix. + */ + if (buf->b_fab_rfm == FAB$C_VFC + || ((buf->b_fab_rat & (FAB$M_FTN | FAB$M_CR)) != 0)) + { + int b2write; + + buf->b_fab_mrs = (buf->b_fab_mrs == 0 + ? MIN(4096, bufsize) + : MIN(buf->b_fab_mrs, bufsize)); + + b2write = len; + while (b2write > 0) + { + write_info.bw_len = MIN(b2write, buf->b_fab_mrs); if (buf_write_bytes(&write_info) == FAIL) { - end = 0; /* write error: break loop */ + end = 0; break; } - nchars += bufsize; - s = buffer; - len = 0; + b2write -= MIN(b2write, buf->b_fab_mrs); } - *s++ = NL; - } - } - if (++len == bufsize && end) - { + write_info.bw_len = bufsize; + nchars += len; + s = buffer; + len = 0; + } +#endif + } + if (len > 0 && end > 0) + { + write_info.bw_len = len; if (buf_write_bytes(&write_info) == FAIL) - { - end = 0; /* write error: break loop */ - break; - } - nchars += bufsize; - s = buffer; - len = 0; - - ui_breakcheck(); - if (got_int) - { - end = 0; /* Interrupted, break loop */ - break; - } - } -#ifdef VMS + end = 0; /* write error */ + nchars += len; + } + + /* Stop when writing done or an error was encountered. */ + if (!checking_conversion || end == 0) + break; + + /* If no error happened until now, writing should be ok, so loop to + * really write the buffer. */ + } + + /* If we started writing, finish writing. Also when an error was + * encountered. */ + if (!checking_conversion) + { +#if defined(UNIX) && defined(HAVE_FSYNC) /* - * On VMS there is a problem: newlines get added when writing blocks - * at a time. Fix it by writing a line at a time. - * This is much slower! - * Explanation: VAX/DECC RTL insists that records in some RMS - * structures end with a newline (carriage return) character, and if - * they don't it adds one. - * With other RMS structures it works perfect without this fix. + * On many journalling file systems there is a bug that causes both the + * original and the backup file to be lost when halting the system + * right after writing the file. That's because only the meta-data is + * journalled. Syncing the file slows down the system, but assures it + * has been written to disk and we don't lose it. + * For a device do try the fsync() but don't complain if it does not + * work (could be a pipe). + * If the 'fsync' option is FALSE, don't fsync(). Useful for laptops. */ - if (buf->b_fab_rfm == FAB$C_VFC - || ((buf->b_fab_rat & (FAB$M_FTN | FAB$M_CR)) != 0)) - { - int b2write; - - buf->b_fab_mrs = (buf->b_fab_mrs == 0 - ? MIN(4096, bufsize) - : MIN(buf->b_fab_mrs, bufsize)); - - b2write = len; - while (b2write > 0) - { - write_info.bw_len = MIN(b2write, buf->b_fab_mrs); - if (buf_write_bytes(&write_info) == FAIL) - { - end = 0; - break; - } - b2write -= MIN(b2write, buf->b_fab_mrs); - } - write_info.bw_len = bufsize; - nchars += len; - s = buffer; - len = 0; - } -#endif - } - if (len > 0 && end > 0) - { - write_info.bw_len = len; - if (buf_write_bytes(&write_info) == FAIL) - end = 0; /* write error */ - nchars += len; - } - -#if defined(UNIX) && defined(HAVE_FSYNC) - /* On many journalling file systems there is a bug that causes both the - * original and the backup file to be lost when halting the system right - * after writing the file. That's because only the meta-data is - * journalled. Syncing the file slows down the system, but assures it has - * been written to disk and we don't lose it. - * For a device do try the fsync() but don't complain if it does not work - * (could be a pipe). - * If the 'fsync' option is FALSE, don't fsync(). Useful for laptops. */ - if (p_fs && fsync(fd) != 0 && !device) - { - errmsg = (char_u *)_("E667: Fsync failed"); - end = 0; - } + if (p_fs && fsync(fd) != 0 && !device) + { + errmsg = (char_u *)_("E667: Fsync failed"); + end = 0; + } #endif #if defined(HAVE_SELINUX) || defined(HAVE_SMACK) - /* Probably need to set the security context. */ - if (!backup_copy) - mch_copy_sec(backup, wfname); + /* Probably need to set the security context. */ + if (!backup_copy) + mch_copy_sec(backup, wfname); #endif #ifdef UNIX - /* When creating a new file, set its owner/group to that of the original - * file. Get the new device and inode number. */ - if (backup != NULL && !backup_copy) - { + /* When creating a new file, set its owner/group to that of the + * original file. Get the new device and inode number. */ + if (backup != NULL && !backup_copy) + { # ifdef HAVE_FCHOWN - stat_T st; - - /* don't change the owner when it's already OK, some systems remove - * permission or ACL stuff */ - if (mch_stat((char *)wfname, &st) < 0 - || st.st_uid != st_old.st_uid - || st.st_gid != st_old.st_gid) - { - ignored = fchown(fd, st_old.st_uid, st_old.st_gid); - if (perm >= 0) /* set permission again, may have changed */ - (void)mch_setperm(wfname, perm); - } + stat_T st; + + /* don't change the owner when it's already OK, some systems remove + * permission or ACL stuff */ + if (mch_stat((char *)wfname, &st) < 0 + || st.st_uid != st_old.st_uid + || st.st_gid != st_old.st_gid) + { + ignored = fchown(fd, st_old.st_uid, st_old.st_gid); + if (perm >= 0) /* set permission again, may have changed */ + (void)mch_setperm(wfname, perm); + } # endif - buf_setino(buf); - } - else if (!buf->b_dev_valid) - /* Set the inode when creating a new file. */ - buf_setino(buf); -#endif - - if (close(fd) != 0) - { - errmsg = (char_u *)_("E512: Close failed"); - end = 0; - } + buf_setino(buf); + } + else if (!buf->b_dev_valid) + /* Set the inode when creating a new file. */ + buf_setino(buf); +#endif + + if (close(fd) != 0) + { + errmsg = (char_u *)_("E512: Close failed"); + end = 0; + } #ifdef UNIX - if (made_writable) - perm &= ~0200; /* reset 'w' bit for security reasons */ -#endif - if (perm >= 0) /* set perm. of new file same as old file */ - (void)mch_setperm(wfname, perm); + if (made_writable) + perm &= ~0200; /* reset 'w' bit for security reasons */ +#endif + if (perm >= 0) /* set perm. of new file same as old file */ + (void)mch_setperm(wfname, perm); #ifdef HAVE_ACL - /* - * Probably need to set the ACL before changing the user (can't set the - * ACL on a file the user doesn't own). - * On Solaris, with ZFS and the aclmode property set to "discard" (the - * default), chmod() discards all part of a file's ACL that don't represent - * the mode of the file. It's non-trivial for us to discover whether we're - * in that situation, so we simply always re-set the ACL. - */ + /* + * Probably need to set the ACL before changing the user (can't set the + * ACL on a file the user doesn't own). + * On Solaris, with ZFS and the aclmode property set to "discard" (the + * default), chmod() discards all part of a file's ACL that don't + * represent the mode of the file. It's non-trivial for us to discover + * whether we're in that situation, so we simply always re-set the ACL. + */ # ifndef HAVE_SOLARIS_ZFS_ACL - if (!backup_copy) + if (!backup_copy) # endif - mch_set_acl(wfname, acl); + mch_set_acl(wfname, acl); #endif #ifdef FEAT_CRYPT - if (buf->b_cryptstate != NULL) - { - crypt_free_state(buf->b_cryptstate); - buf->b_cryptstate = NULL; - } + if (buf->b_cryptstate != NULL) + { + crypt_free_state(buf->b_cryptstate); + buf->b_cryptstate = NULL; + } #endif #if defined(FEAT_MBYTE) && defined(FEAT_EVAL) - if (wfname != fname) + if (wfname != fname) + { + /* + * The file was written to a temp file, now it needs to be + * converted with 'charconvert' to (overwrite) the output file. + */ + if (end != 0) + { + if (eval_charconvert(enc_utf8 ? (char_u *)"utf-8" : p_enc, + fenc, wfname, fname) == FAIL) + { + write_info.bw_conv_error = TRUE; + end = 0; + } + } + mch_remove(wfname); + vim_free(wfname); + } +#endif + } + + if (end == 0) { /* - * The file was written to a temp file, now it needs to be converted - * with 'charconvert' to (overwrite) the output file. + * Error encountered. */ - if (end != 0) - { - if (eval_charconvert(enc_utf8 ? (char_u *)"utf-8" : p_enc, fenc, - wfname, fname) == FAIL) - { - write_info.bw_conv_error = TRUE; - end = 0; - } - } - mch_remove(wfname); - vim_free(wfname); - } -#endif - - if (end == 0) - { if (errmsg == NULL) { #ifdef FEAT_MBYTE @@ -5690,6 +5749,10 @@ buf_write_bytes(struct bw_info *ip) } #endif /* FEAT_MBYTE */ + if (ip->bw_fd < 0) + /* Only checking conversion, which is OK if we get here. */ + return OK; + #ifdef FEAT_CRYPT if (flags & FIO_ENCRYPTED) {
--- a/src/testdir/test_writefile.vim +++ b/src/testdir/test_writefile.vim @@ -31,3 +31,21 @@ func Test_writefile_fails_gently() call assert_fails('call writefile([], [])', 'E730:') endfunc + +func Test_writefile_fails_conversion() + if !has('multi_byte') || !has('iconv') + return + endif + set nobackup nowritebackup + new + let contents = ["line one", "line two"] + call writefile(contents, 'Xfile') + edit Xfile + call setline(1, ["first line", "cannot convert \u010b", "third line"]) + call assert_fails('write ++enc=cp932') + call assert_equal(contents, readfile('Xfile')) + + call delete('Xfile') + bwipe! + set backup& writebackup& +endfunc