Skip to content

Commit d7e43d4

Browse files
Adding missing file closes, rewriting safeWrite() to be more robust (zyedidia#3807)
1 parent dbdf753 commit d7e43d4

File tree

2 files changed

+38
-21
lines changed

2 files changed

+38
-21
lines changed

internal/buffer/buffer.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,7 @@ func (b *Buffer) ReOpen() error {
537537
if err != nil {
538538
return err
539539
}
540+
defer file.Close()
540541

541542
enc, err := htmlindex.Get(b.Settings["encoding"].(string))
542543
if err != nil {

internal/buffer/save.go

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,27 @@ func (b *Buffer) saveToFile(filename string, withSudo bool, autoSave bool) error
337337
return err
338338
}
339339

340+
func (b *Buffer) writeBackup(path string) (string, error) {
341+
backupDir := b.backupDir()
342+
if _, err := os.Stat(backupDir); err != nil {
343+
if !errors.Is(err, fs.ErrNotExist) {
344+
return "", err
345+
}
346+
if err = os.Mkdir(backupDir, os.ModePerm); err != nil {
347+
return "", err
348+
}
349+
}
350+
351+
backupName := util.DetermineEscapePath(backupDir, path)
352+
_, err := b.overwriteFile(backupName)
353+
if err != nil {
354+
os.Remove(backupName)
355+
return "", err
356+
}
357+
358+
return backupName, nil
359+
}
360+
340361
// safeWrite writes the buffer to a file in a "safe" way, preventing loss of the
341362
// contents of the file if it fails to write the new contents.
342363
// This means that the file is not overwritten directly but by writing to the
@@ -353,39 +374,34 @@ func (b *Buffer) safeWrite(path string, withSudo bool, newFile bool) (int, error
353374
}
354375
}()
355376

356-
backupDir := b.backupDir()
357-
if _, err := os.Stat(backupDir); err != nil {
358-
if !errors.Is(err, fs.ErrNotExist) {
359-
return 0, err
360-
}
361-
if err = os.Mkdir(backupDir, os.ModePerm); err != nil {
362-
return 0, err
363-
}
364-
}
365377

366-
backupName := util.DetermineEscapePath(backupDir, path)
367-
_, err = b.overwriteFile(backupName)
378+
// Try to backup first before writing
379+
backupName, err := b.writeBackup(path)
368380
if err != nil {
369-
os.Remove(backupName)
381+
file.Close()
370382
return 0, err
371383
}
372384

373385
b.forceKeepBackup = true
374-
size, err := file.Write(b)
375-
if err != nil {
376-
err = util.OverwriteError{err, backupName}
377-
return size, err
386+
size := 0
387+
{
388+
// If we failed to write or close, keep the backup we made
389+
size, err = file.Write(b)
390+
if err != nil {
391+
file.Close()
392+
return 0, util.OverwriteError{err, backupName}
393+
}
394+
395+
err = file.Close()
396+
if err != nil {
397+
return 0, util.OverwriteError{err, backupName}
398+
}
378399
}
379400
b.forceKeepBackup = false
380401

381402
if !b.keepBackup() {
382403
os.Remove(backupName)
383404
}
384405

385-
err2 := file.Close()
386-
if err2 != nil && err == nil {
387-
err = err2
388-
}
389-
390406
return size, err
391407
}

0 commit comments

Comments
 (0)