Skip to content

Commit 2eefbb5

Browse files
tenthirtyamtenthirtyam
authored
refactor: handle ignored file operation errors (#545)
Replace unchecked Close/Remove/Write-style calls with explicit error handling or intentional ignored-error wrappers. Runtime paths now propagate meaningful errors where it matters and cleanup paths either log failures or explicitly discard errors to satisfy linting while preserving behavior. Signed-off-by: Ryan Johnson <ryan@tenthirtyam.org>
1 parent 0ed4a31 commit 2eefbb5

22 files changed

Lines changed: 112 additions & 76 deletions

‎builder/vmware/common/driver.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ func ReadNetmapConfig(path string) (NetworkMap, error) {
432432
if err != nil {
433433
return nil, err
434434
}
435-
defer fd.Close()
435+
defer func() { _ = fd.Close() }()
436436
return ReadNetworkMap(fd)
437437
}
438438

@@ -442,7 +442,7 @@ func ReadDhcpConfig(path string) (DhcpConfiguration, error) {
442442
if err != nil {
443443
return nil, err
444444
}
445-
defer fd.Close()
445+
defer func() { _ = fd.Close() }()
446446
return ReadDhcpConfiguration(fd)
447447
}
448448

@@ -452,7 +452,7 @@ func readVMXConfig(path string) (map[string]string, error) {
452452
if err != nil {
453453
return map[string]string{}, err
454454
}
455-
defer f.Close()
455+
defer func() { _ = f.Close() }()
456456

457457
vmxBytes, err := io.ReadAll(f)
458458
if err != nil {
@@ -730,7 +730,7 @@ func (d *VmwareDriver) HostIP(state multistep.StateBag) (string, error) {
730730
func getHostIPForBridgedNetwork() (string, error) {
731731
// Determine the source IP used for default-route communications.
732732
if conn, err := net.Dial("udp4", "1.1.1.1:53"); err == nil {
733-
defer conn.Close()
733+
defer func() { _ = conn.Close() }()
734734
if addr, ok := conn.LocalAddr().(*net.UDPAddr); ok {
735735
log.Printf("[INFO] Discovered host IP address for bridged network using default route: %s", addr.IP)
736736
return addr.IP.String(), nil

‎builder/vmware/common/driver_fusion.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ func (d *FusionDriver) Verify() error {
299299
if err != nil {
300300
return nil, err
301301
}
302-
defer fd.Close()
302+
defer func() { _ = fd.Close() }()
303303

304304
return ReadNetworkingConfig(fd)
305305
}

‎builder/vmware/common/driver_parser_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ parameters : map[default-lease-time:1800 max-lease-time:7200]
352352
if err != nil {
353353
t.Fatalf("Failed to open dhcpd.conf sample: %s", err)
354354
}
355-
defer f.Close()
355+
defer func() { _ = f.Close() }()
356356

357357
config, err := ReadDhcpConfiguration(f)
358358
if err != nil {
@@ -422,7 +422,7 @@ func TestParserReadNetworkMap(t *testing.T) {
422422
if err != nil {
423423
t.Fatalf("Unable to open netmap.conf sample: %s", err)
424424
}
425-
defer f.Close()
425+
defer func() { _ = f.Close() }()
426426

427427
netmap, err := ReadNetworkMap(f)
428428
if err != nil {
@@ -760,7 +760,7 @@ func TestParserReadDhcpdLeases(t *testing.T) {
760760
if err != nil {
761761
t.Fatalf("Unable to open dhcpd.leases sample: %s", err)
762762
}
763-
defer f.Close()
763+
defer func() { _ = f.Close() }()
764764

765765
results, err := ReadDhcpdLeaseEntries(f)
766766
if err != nil {
@@ -945,7 +945,7 @@ func TestParserReadAppleDhcpdLeases(t *testing.T) {
945945
if err != nil {
946946
t.Fatalf("Unable to open dhcpd.leases sample: %s", err)
947947
}
948-
defer f.Close()
948+
defer func() { _ = f.Close() }()
949949

950950
results, err := ReadAppleDhcpdLeaseEntries(f)
951951
if err != nil {
@@ -1211,7 +1211,7 @@ func TestParserReadNetworingConfig(t *testing.T) {
12111211
if err != nil {
12121212
t.Fatalf("Unable to open networking-example sample: %s", err)
12131213
}
1214-
defer f.Close()
1214+
defer func() { _ = f.Close() }()
12151215

12161216
config, err := ReadNetworkingConfig(f)
12171217
if err != nil {

‎builder/vmware/common/driver_workstation.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ func checkNetmapConfExists() (NetworkNameMapper, error) {
360360
return nil, err
361361
}
362362

363-
defer fd.Close()
363+
defer func() { _ = fd.Close() }()
364364

365365
// Pass the handle to the networking configuration parser.
366366
return ReadNetworkingConfig(fd)

‎builder/vmware/common/ssh_config_test.go‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ func TestSSHConfigPrepare_SSHPrivateKey(t *testing.T) {
5757
if err != nil {
5858
t.Fatalf("err: %s", err)
5959
}
60-
defer os.Remove(tf.Name())
61-
defer tf.Close()
60+
defer func() { _ = os.Remove(tf.Name()) }()
61+
defer func() { _ = tf.Close() }()
6262

6363
if _, err := tf.Write([]byte("HELLO!")); err != nil {
6464
t.Fatalf("err: %s", err)
@@ -72,12 +72,15 @@ func TestSSHConfigPrepare_SSHPrivateKey(t *testing.T) {
7272
}
7373

7474
// Test good contents
75-
//nolint
76-
tf.Seek(0, 0)
77-
//nolint
78-
tf.Truncate(0)
79-
//nolint
80-
tf.Write([]byte(testPem))
75+
if _, err := tf.Seek(0, 0); err != nil {
76+
t.Fatalf("err: %s", err)
77+
}
78+
if err := tf.Truncate(0); err != nil {
79+
t.Fatalf("err: %s", err)
80+
}
81+
if _, err := tf.Write([]byte(testPem)); err != nil {
82+
t.Fatalf("err: %s", err)
83+
}
8184
c = testSSHConfig()
8285
c.Comm.SSHPrivateKeyFile = tf.Name()
8386
errs = c.Prepare(interpolate.NewContext())

‎builder/vmware/common/step_clean_vmx_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func TestStepCleanVMX(t *testing.T) {
2121
step := new(StepCleanVMX)
2222

2323
vmxPath := testVMXFile(t)
24-
defer os.Remove(vmxPath)
24+
defer func() { _ = os.Remove(vmxPath) }()
2525
state.Put("vmx_path", vmxPath)
2626

2727
// Test the run
@@ -38,7 +38,7 @@ func TestStepCleanVMX_floppyPath(t *testing.T) {
3838
step := new(StepCleanVMX)
3939

4040
vmxPath := testVMXFile(t)
41-
defer os.Remove(vmxPath)
41+
defer func() { _ = os.Remove(vmxPath) }()
4242
if err := os.WriteFile(vmxPath, []byte(testVMXFloppyPath), 0644); err != nil { //nolint:gosec
4343
t.Fatalf("err: %s", err)
4444
}
@@ -91,7 +91,7 @@ func TestStepCleanVMX_isoPath(t *testing.T) {
9191
step := new(StepCleanVMX)
9292

9393
vmxPath := testVMXFile(t)
94-
defer os.Remove(vmxPath)
94+
defer func() { _ = os.Remove(vmxPath) }()
9595
if err := os.WriteFile(vmxPath, []byte(testVMXISOPath), 0644); err != nil { //nolint:gosec
9696
t.Fatalf("err: %s", err)
9797
}
@@ -147,7 +147,7 @@ func TestStepCleanVMX_ethernet(t *testing.T) {
147147
}
148148

149149
vmxPath := testVMXFile(t)
150-
defer os.Remove(vmxPath)
150+
defer func() { _ = os.Remove(vmxPath) }()
151151
if err := os.WriteFile(vmxPath, []byte(testVMXEthernet), 0644); err != nil { //nolint:gosec
152152
t.Fatalf("err: %s", err)
153153
}
@@ -226,7 +226,7 @@ func TestStepCleanVMX_toolsCDROM(t *testing.T) {
226226
step := new(StepCleanVMX)
227227

228228
vmxPath := testVMXFile(t)
229-
defer os.Remove(vmxPath)
229+
defer func() { _ = os.Remove(vmxPath) }()
230230
if err := os.WriteFile(vmxPath, []byte(testVMXToolsCDROM), 0644); err != nil { //nolint:gosec
231231
t.Fatalf("err: %s", err)
232232
}
@@ -292,7 +292,7 @@ func TestStepCleanVMX_toolsCDROMOnly(t *testing.T) {
292292
step := new(StepCleanVMX)
293293

294294
vmxPath := testVMXFile(t)
295-
defer os.Remove(vmxPath)
295+
defer func() { _ = os.Remove(vmxPath) }()
296296
if err := os.WriteFile(vmxPath, []byte(testVMXToolsCDROMOnly), 0644); err != nil { //nolint:gosec
297297
t.Fatalf("err: %s", err)
298298
}
@@ -352,7 +352,7 @@ func TestStepCleanVMX_noToolsCDROM(t *testing.T) {
352352
step := new(StepCleanVMX)
353353

354354
vmxPath := testVMXFile(t)
355-
defer os.Remove(vmxPath)
355+
defer func() { _ = os.Remove(vmxPath) }()
356356
if err := os.WriteFile(vmxPath, []byte(testVMXISOPath), 0644); err != nil { //nolint:gosec
357357
t.Fatalf("err: %s", err)
358358
}
@@ -403,7 +403,7 @@ func TestStepCleanVMX_preserveUserCDROM(t *testing.T) {
403403
step := new(StepCleanVMX)
404404

405405
vmxPath := testVMXFile(t)
406-
defer os.Remove(vmxPath)
406+
defer func() { _ = os.Remove(vmxPath) }()
407407
if err := os.WriteFile(vmxPath, []byte(testVMXMultipleCDROM), 0644); err != nil { //nolint:gosec
408408
t.Fatalf("err: %s", err)
409409
}

‎builder/vmware/common/step_configure_vmx_test.go‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ func testVMXFile(t *testing.T) string {
2424
if err != nil {
2525
t.Fatalf("error writing .vmx file: %s", err)
2626
}
27-
tf.Close()
27+
if err := tf.Close(); err != nil {
28+
t.Fatalf("err: %s", err)
29+
}
2830

2931
return tf.Name()
3032
}
@@ -41,7 +43,7 @@ func TestStepConfigureVMX(t *testing.T) {
4143
}
4244

4345
vmxPath := testVMXFile(t)
44-
defer os.Remove(vmxPath)
46+
defer func() { _ = os.Remove(vmxPath) }()
4547
state.Put("vmx_path", vmxPath)
4648

4749
// Test the run
@@ -92,7 +94,7 @@ func TestStepConfigureVMX_floppyPath(t *testing.T) {
9294
step := new(StepConfigureVMX)
9395

9496
vmxPath := testVMXFile(t)
95-
defer os.Remove(vmxPath)
97+
defer func() { _ = os.Remove(vmxPath) }()
9698

9799
state.Put("floppy_path", "foo")
98100
state.Put("vmx_path", vmxPath)
@@ -140,7 +142,7 @@ func TestStepConfigureVMX_generatedAddresses(t *testing.T) {
140142
step := new(StepConfigureVMX)
141143

142144
vmxPath := testVMXFile(t)
143-
defer os.Remove(vmxPath)
145+
defer func() { _ = os.Remove(vmxPath) }()
144146

145147
additionalTestVmxData := []struct {
146148
Key string
@@ -216,7 +218,7 @@ func TestStepConfigureVMX_displayNameMissing(t *testing.T) {
216218

217219
// testVMXFile adds displayName key/value pair to the VMX
218220
vmxPath := testVMXFile(t)
219-
defer os.Remove(vmxPath)
221+
defer func() { _ = os.Remove(vmxPath) }()
220222

221223
// Bad: Delete displayName from the VMX/Create an empty VMX file
222224
err := WriteVMX(vmxPath, map[string]string{})
@@ -242,7 +244,7 @@ func TestStepConfigureVMX_displayNameStore(t *testing.T) {
242244

243245
// testVMXFile adds displayName key/value pair to the VMX
244246
vmxPath := testVMXFile(t)
245-
defer os.Remove(vmxPath)
247+
defer func() { _ = os.Remove(vmxPath) }()
246248

247249
state.Put("vmx_path", vmxPath)
248250

@@ -388,7 +390,7 @@ func TestStepConfigureVMX_CDROMAdapter(t *testing.T) {
388390
}
389391

390392
vmxPath := testVMXFile(t)
391-
defer os.Remove(vmxPath)
393+
defer func() { _ = os.Remove(vmxPath) }()
392394
state.Put("vmx_path", vmxPath)
393395
state.Put("temporaryDevices", []string{})
394396

‎builder/vmware/common/step_configure_vnc.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ func (s *StepConfigureVNC) VNCAddress(ctx context.Context, vncBindAddress string
5252
return "", 0, err
5353
}
5454

55-
s.l.Listener.Close() // free port, but don't unlock lock file
55+
// Free the port, but don't unlock the lock file.
56+
if err := s.l.Listener.Close(); err != nil {
57+
return "", 0, err
58+
}
5659
return s.l.Address, s.l.Port, nil
5760
}
5861

‎builder/vmware/common/step_output_dir_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func testOutputDir(t *testing.T) string {
1717
if err != nil {
1818
t.Fatalf("err: %s", err)
1919
}
20-
os.RemoveAll(td)
20+
_ = os.RemoveAll(td)
2121

2222
return td
2323
}
@@ -41,7 +41,7 @@ func TestStepOutputDir(t *testing.T) {
4141
VMName: "testVM",
4242
}
4343
// Delete the test output directory when done
44-
defer os.RemoveAll(td)
44+
defer func() { _ = os.RemoveAll(td) }()
4545

4646
// Test the run
4747
if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
@@ -74,13 +74,13 @@ func TestStepOutputDir_existsNoForce(t *testing.T) {
7474
VMName: "testVM",
7575
}
7676
// Delete the test output directory when done
77-
defer os.RemoveAll(td)
77+
defer func() { _ = os.RemoveAll(td) }()
7878

7979
// Make sure the dir exists
8080
if err := os.MkdirAll(td, 0755); err != nil {
8181
t.Fatalf("err: %s", err)
8282
}
83-
defer os.RemoveAll(td)
83+
defer func() { _ = os.RemoveAll(td) }()
8484

8585
// Test the run
8686
if action := step.Run(context.Background(), state); action != multistep.ActionHalt {
@@ -112,13 +112,13 @@ func TestStepOutputDir_existsForce(t *testing.T) {
112112
step.Force = true
113113

114114
// Delete the test output directory when done
115-
defer os.RemoveAll(td)
115+
defer func() { _ = os.RemoveAll(td) }()
116116

117117
// Make sure the dir exists
118118
if err := os.MkdirAll(td, 0755); err != nil {
119119
t.Fatalf("err: %s", err)
120120
}
121-
defer os.RemoveAll(td)
121+
defer func() { _ = os.RemoveAll(td) }()
122122

123123
// Test the run
124124
if action := step.Run(context.Background(), state); action != multistep.ActionContinue {

‎builder/vmware/common/step_prepare_tools_test.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ func TestStepPrepareTools(t *testing.T) {
2121
if err != nil {
2222
t.Fatalf("err: %s", err)
2323
}
24-
tf.Close()
25-
defer os.Remove(tf.Name())
24+
if err := tf.Close(); err != nil {
25+
t.Fatalf("err: %s", err)
26+
}
27+
defer func() { _ = os.Remove(tf.Name()) }()
2628

2729
state := testState(t)
2830
step := &StepPrepareTools{
@@ -195,8 +197,10 @@ func TestStepPrepareTools_AttachMode_Flavor(t *testing.T) {
195197
if err != nil {
196198
t.Fatalf("err: %s", err)
197199
}
198-
tf.Close()
199-
defer os.Remove(tf.Name())
200+
if err := tf.Close(); err != nil {
201+
t.Fatalf("err: %s", err)
202+
}
203+
defer func() { _ = os.Remove(tf.Name()) }()
200204

201205
state := testState(t)
202206
step := &StepPrepareTools{
@@ -275,8 +279,10 @@ func TestStepPrepareTools_BackwardCompatibility(t *testing.T) {
275279
if err != nil {
276280
t.Fatalf("err: %s", err)
277281
}
278-
tf.Close()
279-
defer os.Remove(tf.Name())
282+
if err := tf.Close(); err != nil {
283+
t.Fatalf("err: %s", err)
284+
}
285+
defer func() { _ = os.Remove(tf.Name()) }()
280286

281287
state := testState(t)
282288
step := &StepPrepareTools{

0 commit comments

Comments
 (0)