Skip to content

Commit a2f55b9

Browse files
RuoqingHeroypat
authored andcommitted
chore: Eliminate use of assert!((...).is_ok())
As @roypat pointed out, and quote: "Asserting on .is_ok()/.is_err() leads to hard to debug failures (as if the test fails, it will only say "assertion failed: false". We replace these with `.unwrap()`, which also prints the exact error variant that was unexpectedly encountered (we can to this these days thanks to efforts to implement Display and Debug for our error types). If the assert!((...).is_ok()) was followed by an .unwrap() anyway, we just drop the assert." Signed-off-by: Ruoqing He <[email protected]>
1 parent 5e82393 commit a2f55b9

File tree

4 files changed

+130
-131
lines changed

4 files changed

+130
-131
lines changed

src/ioctls/device.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ mod tests {
212212
flags: KVM_CREATE_DEVICE_TEST,
213213
};
214214
// This fails on x86_64 because there is no VGIC there.
215-
assert!(vm.create_device(&mut gic_device).is_err());
215+
vm.create_device(&mut gic_device).unwrap_err();
216216

217217
gic_device.type_ = kvm_device_type_KVM_DEV_TYPE_VFIO;
218218

@@ -237,9 +237,9 @@ mod tests {
237237

238238
// We are just creating a test device. Creating a real device would make the CI dependent
239239
// on host configuration (like having /dev/vfio). We expect this to fail.
240-
assert!(device_fd.has_device_attr(&dist_attr).is_err());
241-
assert!(unsafe { device_fd.get_device_attr(&mut dist_attr_mut) }.is_err());
242-
assert!(device_fd.set_device_attr(&dist_attr).is_err());
240+
device_fd.has_device_attr(&dist_attr).unwrap_err();
241+
unsafe { device_fd.get_device_attr(&mut dist_attr_mut) }.unwrap_err();
242+
device_fd.set_device_attr(&dist_attr).unwrap_err();
243243
assert_eq!(errno::Error::last().errno(), 25);
244244
}
245245

@@ -262,7 +262,7 @@ mod tests {
262262
};
263263
// This fails on aarch64 as it does not use MPIC (MultiProcessor Interrupt Controller),
264264
// it uses the VGIC.
265-
assert!(vm.create_device(&mut gic_device).is_err());
265+
vm.create_device(&mut gic_device).unwrap_err();
266266

267267
let device_fd = create_gic_device(&vm, 0);
268268

@@ -283,7 +283,7 @@ mod tests {
283283
addr: 0x0,
284284
flags: 0,
285285
};
286-
assert!(device_fd.has_device_attr(&dist_attr).is_err());
286+
device_fd.has_device_attr(&dist_attr).unwrap_err();
287287

288288
// Set maximum supported number of IRQs of the vGIC device to 128.
289289
set_supported_nr_irqs(&device_fd, 128);
@@ -307,7 +307,7 @@ mod tests {
307307
assert_eq!(res, Err(Error::new(libc::EFAULT)));
308308

309309
gic_attr.addr = &mut data as *mut u32 as u64;
310-
assert!(unsafe { device_fd.get_device_attr(&mut gic_attr) }.is_ok());
310+
unsafe { device_fd.get_device_attr(&mut gic_attr) }.unwrap();
311311
// The maximum supported number of IRQs should be 128, same as the value
312312
// when we initialize the GIC.
313313
assert_eq!(data, 128);

src/ioctls/system.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -857,14 +857,13 @@ mod tests {
857857
kvm.create_vm_with_ipa_size(host_ipa_limit as u32).unwrap();
858858
// Test invalid input values
859859
// Case 1: IPA size is smaller than 32.
860-
assert!(kvm.create_vm_with_ipa_size(31).is_err());
860+
kvm.create_vm_with_ipa_size(31).unwrap_err();
861861
// Case 2: IPA size is bigger than Host_IPA_Limit.
862-
assert!(kvm
863-
.create_vm_with_ipa_size((host_ipa_limit + 1) as u32)
864-
.is_err());
862+
kvm.create_vm_with_ipa_size((host_ipa_limit + 1) as u32)
863+
.unwrap_err();
865864
} else {
866865
// Unsupported, we can't provide an IPA size. Only KVM type=0 works.
867-
assert!(kvm.create_vm_with_type(0).is_err());
866+
kvm.create_vm_with_type(0).unwrap_err();
868867
}
869868
}
870869

@@ -879,7 +878,7 @@ mod tests {
879878

880879
// Test case for more than MAX entries
881880
let cpuid_err = kvm.get_emulated_cpuid(KVM_MAX_CPUID_ENTRIES + 1_usize);
882-
assert!(cpuid_err.is_err());
881+
cpuid_err.unwrap_err();
883882
}
884883

885884
#[test]
@@ -893,7 +892,7 @@ mod tests {
893892

894893
// Test case for more than MAX entries
895894
let cpuid_err = kvm.get_emulated_cpuid(KVM_MAX_CPUID_ENTRIES + 1_usize);
896-
assert!(cpuid_err.is_err());
895+
cpuid_err.unwrap_err();
897896
}
898897

899898
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]

src/ioctls/vcpu.rs

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2009,7 +2009,7 @@ mod tests {
20092009
let kvm = Kvm::new().unwrap();
20102010
let vm = kvm.create_vm().unwrap();
20112011

2012-
assert!(vm.create_vcpu(0).is_ok());
2012+
vm.create_vcpu(0).unwrap();
20132013
}
20142014

20152015
#[cfg(target_arch = "x86_64")]
@@ -2154,7 +2154,7 @@ mod tests {
21542154
assert!(kvm.check_extension(Cap::Irqchip));
21552155
let vm = kvm.create_vm().unwrap();
21562156
// The get_lapic ioctl will fail if there is no irqchip created beforehand.
2157-
assert!(vm.create_irq_chip().is_ok());
2157+
vm.create_irq_chip().unwrap();
21582158
let vcpu = vm.create_vcpu(0).unwrap();
21592159
let mut klapic: kvm_lapic_state = vcpu.get_lapic().unwrap();
21602160

@@ -2636,7 +2636,7 @@ mod tests {
26362636
);
26372637
// `kvm_lapic_state` does not implement debug by default so we cannot
26382638
// use unwrap_err here.
2639-
assert!(faulty_vcpu_fd.get_lapic().is_err());
2639+
faulty_vcpu_fd.get_lapic().unwrap_err();
26402640
assert_eq!(
26412641
faulty_vcpu_fd
26422642
.set_lapic(&unsafe { std::mem::zeroed() })
@@ -2693,9 +2693,9 @@ mod tests {
26932693
faulty_vcpu_fd.kvmclock_ctrl().unwrap_err().errno(),
26942694
badf_errno
26952695
);
2696-
assert!(faulty_vcpu_fd.get_tsc_khz().is_err());
2697-
assert!(faulty_vcpu_fd.set_tsc_khz(1000000).is_err());
2698-
assert!(faulty_vcpu_fd.translate_gva(u64::MAX).is_err());
2696+
faulty_vcpu_fd.get_tsc_khz().unwrap_err();
2697+
faulty_vcpu_fd.set_tsc_khz(1000000).unwrap_err();
2698+
faulty_vcpu_fd.translate_gva(u64::MAX).unwrap_err();
26992699

27002700
// Don't drop the File object, or it'll notice the file it's trying to close is
27012701
// invalid and abort the process.
@@ -2716,7 +2716,7 @@ mod tests {
27162716
..Default::default()
27172717
};
27182718

2719-
assert!(vcpu.vcpu_init(&kvi).is_err());
2719+
vcpu.vcpu_init(&kvi).unwrap_err();
27202720
}
27212721

27222722
#[test]
@@ -2811,7 +2811,7 @@ mod tests {
28112811

28122812
vm.get_preferred_target(&mut kvi)
28132813
.expect("Cannot get preferred target");
2814-
assert!(vcpu.vcpu_init(&kvi).is_ok());
2814+
vcpu.vcpu_init(&kvi).unwrap();
28152815
}
28162816

28172817
#[test]
@@ -2828,7 +2828,7 @@ mod tests {
28282828
let data: u128 = 0;
28292829
let reg_id: u64 = 0;
28302830

2831-
assert!(vcpu.set_one_reg(reg_id, &data.to_le_bytes()).is_err());
2831+
vcpu.set_one_reg(reg_id, &data.to_le_bytes()).unwrap_err();
28322832
// Exercising KVM_SET_ONE_REG by trying to alter the data inside the PSTATE register (which is a
28332833
// specific aarch64 register).
28342834
// This regiseter is 64 bit wide (8 bytes).
@@ -2837,7 +2837,7 @@ mod tests {
28372837
.expect("Failed to set pstate register");
28382838

28392839
// Trying to set 8 byte register with 7 bytes must fail.
2840-
assert!(vcpu.set_one_reg(PSTATE_REG_ID, &[0_u8; 7]).is_err());
2840+
vcpu.set_one_reg(PSTATE_REG_ID, &[0_u8; 7]).unwrap_err();
28412841
}
28422842

28432843
#[test]
@@ -2873,7 +2873,7 @@ mod tests {
28732873
assert_eq!(data, PSTATE_FAULT_BITS_64 as u128);
28742874

28752875
// Trying to get 8 byte register with 7 bytes must fail.
2876-
assert!(vcpu.get_one_reg(PSTATE_REG_ID, &mut [0_u8; 7]).is_err());
2876+
vcpu.get_one_reg(PSTATE_REG_ID, &mut [0_u8; 7]).unwrap_err();
28772877
}
28782878

28792879
#[test]
@@ -2905,7 +2905,7 @@ mod tests {
29052905
// SAFETY: This structure is a result from a specific vCPU ioctl
29062906
let mut reg_list =
29072907
RegList::new(unsafe { reg_list.as_mut_fam_struct() }.n as usize).unwrap();
2908-
assert!(vcpu.get_reg_list(&mut reg_list).is_ok());
2908+
vcpu.get_reg_list(&mut reg_list).unwrap()
29092909
}
29102910

29112911
#[test]
@@ -2957,7 +2957,7 @@ mod tests {
29572957
let vcpu = vm.create_vcpu(0).unwrap();
29582958

29592959
if !kvm.check_extension(Cap::GetTscKhz) {
2960-
assert!(vcpu.get_tsc_khz().is_err())
2960+
vcpu.get_tsc_khz().unwrap_err();
29612961
} else {
29622962
assert!(vcpu.get_tsc_khz().unwrap() > 0);
29632963
}
@@ -2972,11 +2972,11 @@ mod tests {
29722972
let freq = vcpu.get_tsc_khz().unwrap();
29732973

29742974
if !(kvm.check_extension(Cap::GetTscKhz) && kvm.check_extension(Cap::TscControl)) {
2975-
assert!(vcpu.set_tsc_khz(0).is_err());
2975+
vcpu.set_tsc_khz(0).unwrap_err();
29762976
} else {
2977-
assert!(vcpu.set_tsc_khz(freq - 500000).is_ok());
2977+
vcpu.set_tsc_khz(freq - 500000).unwrap();
29782978
assert_eq!(vcpu.get_tsc_khz().unwrap(), freq - 500000);
2979-
assert!(vcpu.set_tsc_khz(freq + 500000).is_ok());
2979+
vcpu.set_tsc_khz(freq + 500000).unwrap();
29802980
assert_eq!(vcpu.get_tsc_khz().unwrap(), freq + 500000);
29812981
}
29822982
}
@@ -3112,13 +3112,13 @@ mod tests {
31123112
let kvm = Kvm::new().unwrap();
31133113
let vm = kvm.create_vm().unwrap();
31143114
let vcpu = vm.create_vcpu(0).unwrap();
3115-
assert!(vcpu.translate_gva(0x10000).is_ok());
3115+
vcpu.translate_gva(0x10000).unwrap();
31163116
assert_eq!(vcpu.translate_gva(0x10000).unwrap().valid, 1);
31173117
assert_eq!(
31183118
vcpu.translate_gva(0x10000).unwrap().physical_address,
31193119
0x10000
31203120
);
3121-
assert!(vcpu.translate_gva(u64::MAX).is_ok());
3121+
vcpu.translate_gva(u64::MAX).unwrap();
31223122
assert_eq!(vcpu.translate_gva(u64::MAX).unwrap().valid, 0);
31233123
}
31243124

@@ -3136,15 +3136,15 @@ mod tests {
31363136
flags: 0,
31373137
};
31383138

3139-
assert!(vcpu.has_device_attr(&dist_attr).is_err());
3140-
assert!(vcpu.set_device_attr(&dist_attr).is_err());
3139+
vcpu.has_device_attr(&dist_attr).unwrap_err();
3140+
vcpu.set_device_attr(&dist_attr).unwrap_err();
31413141
let mut kvi: kvm_bindings::kvm_vcpu_init = kvm_bindings::kvm_vcpu_init::default();
31423142
vm.get_preferred_target(&mut kvi)
31433143
.expect("Cannot get preferred target");
31443144
kvi.features[0] |= 1 << kvm_bindings::KVM_ARM_VCPU_PSCI_0_2 | 1 << KVM_ARM_VCPU_PMU_V3;
3145-
assert!(vcpu.vcpu_init(&kvi).is_ok());
3146-
assert!(vcpu.has_device_attr(&dist_attr).is_ok());
3147-
assert!(vcpu.set_device_attr(&dist_attr).is_ok());
3145+
vcpu.vcpu_init(&kvi).unwrap();
3146+
vcpu.has_device_attr(&dist_attr).unwrap();
3147+
vcpu.set_device_attr(&dist_attr).unwrap();
31483148
}
31493149

31503150
#[test]
@@ -3163,7 +3163,7 @@ mod tests {
31633163
if kvm.check_extension(Cap::ArmPtrAuthGeneric) {
31643164
kvi.features[0] |= 1 << kvm_bindings::KVM_ARM_VCPU_PTRAUTH_GENERIC;
31653165
}
3166-
assert!(vcpu.vcpu_init(&kvi).is_ok());
3166+
vcpu.vcpu_init(&kvi).unwrap();
31673167
}
31683168

31693169
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]

0 commit comments

Comments
 (0)