Skip to content

Commit 62156c1

Browse files
fix: address review feedback on atomicWriteFile
Signed-off-by: mayanksharmaCSE <mayanksharmacse1@gmail.com>
1 parent b8e3312 commit 62156c1

3 files changed

Lines changed: 26 additions & 3 deletions

File tree

pkg/unikontainers/unikontainers.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,7 @@ func (u *Unikontainer) saveContainerState() error {
706706
}
707707

708708
stateName := filepath.Join(u.BaseDir, stateFilename)
709-
return atomicWriteFile(stateName, data, 0o644) //nolint: gosec
709+
return atomicWriteFile(stateName, data, 0o644)
710710
}
711711

712712
// getHooksByName returns the hooks for a given lifecycle stage

pkg/unikontainers/utils.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func atomicWriteFile(path string, data []byte, perm os.FileMode) error {
114114
dir := filepath.Dir(path)
115115
tmpName := filepath.Join(dir, "."+filepath.Base(path)+".tmp")
116116

117-
f, err := os.OpenFile(tmpName, os.O_RDWR|os.O_CREATE|os.O_TRUNC|os.O_SYNC, perm)
117+
f, err := os.OpenFile(tmpName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, perm)
118118
if err != nil {
119119
return fmt.Errorf("failed to create temp file: %w", err)
120120
}
@@ -136,7 +136,11 @@ func atomicWriteFile(path string, data []byte, perm os.FileMode) error {
136136
return fmt.Errorf("failed to close temp file: %w", closeErr)
137137
}
138138

139-
return os.Rename(tmpName, path)
139+
if err := os.Rename(tmpName, path); err != nil {
140+
os.Remove(tmpName)
141+
return fmt.Errorf("failed to rename temp file to %s: %w", path, err)
142+
}
143+
return nil
140144
}
141145

142146
// writePidFile writes the content of pid to the file defined by path

pkg/unikontainers/utils_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,25 @@ func TestAtomicWriteFile(t *testing.T) {
7777
err := atomicWriteFile(target, []byte("data"), 0o644)
7878
assert.Error(t, err)
7979
})
80+
81+
t.Run("rename failure cleans up temp file", func(t *testing.T) {
82+
t.Parallel()
83+
tmpDir := t.TempDir()
84+
85+
// Create a directory at the target path so rename fails.
86+
target := filepath.Join(tmpDir, "state.json")
87+
err := os.Mkdir(target, 0o755)
88+
assert.NoError(t, err)
89+
90+
err = atomicWriteFile(target, []byte("data"), 0o644)
91+
assert.Error(t, err)
92+
assert.Contains(t, err.Error(), "failed to rename temp file")
93+
94+
// Verify the temp file was cleaned up.
95+
tmpFile := filepath.Join(tmpDir, ".state.json.tmp")
96+
_, err = os.Stat(tmpFile)
97+
assert.True(t, os.IsNotExist(err), "Temp file should be cleaned up after rename failure")
98+
})
8099
}
81100

82101
func TestWritePidFile(t *testing.T) {

0 commit comments

Comments
 (0)