On Tue, Feb 11, 2025 at 11:28:53PM -0500, Laine Stump wrote:
This started with me noticing one function call that checked for failure on something that I knew couldn't fail. So I changed that function to return void. But then I noticed another similar function that also should return void, so I changed that one too. Then eliminating the check for the return from those functions caused another function to become infallible, so I changed that one too, which led to more and more until the evening was finished and I had this long list of tiny mechanical patches.
The classic Hal fixing a lightbulb moment ;)
And here it is - an easy way to improve your review stats :-)
Well, it definitely did not look like it in the list =D
P.S. I know there are more of these, but forced myself to stop here.
Good for you! If you have something in the back of your mind, then feel free to mention it in gitlab issue labelled bitesizedtask, we still mention them and try to give them to newcomers or people who seem interested in starting out.
A related question - is it possible for virObjectNew() to fail? I did finally find (after some searching, documentation that says g_object_new() can't return null, but I don't know enough about g_object stuff to know if the vir*Initialize functions could fail (for example). If virObjectNew() can't fail then that open a whole new can of worms...
It can't... almost, from my point of view. The allocation parts should abort the whole process, but if you want to create a new object of a class which is not of type object, then it will return NULL. That "should not happen", but... you already know how the cookie crumbles. Having said that, we might just abort ourselves and make our own guarantee that virObjectNew() does *not* return NULL, ever.
Laine Stump (17): conf: change virDomainHostdevInsert() to return void conf: change virDomainNetInsert() to return void conf: change virDomainFSInsert() to return void conf: change virDomainShmemDefInsert() to return void conf: change virDomainDefMaybeAddInput() to return void libxl: change xenDomainDefAddImplicitInputDevice() to return void conf: change qemuDomainDefAddImplicitInputDevice() to return void conf: stop checking for NULL return from virDomainControllerDefNew() conf: stop checking for NULL return from virDomainDefAddController() conf: change virDomainDefAddUSBController() to return void hyperv: change hypervDomainDefAppendController() to return void conf: change virDomainDefMaybeAddController() to return true/false conf: change virDomainDefMaybeAddHostdevSCSIcontroller() to return void conf: change virDomainDefAddDiskControllersForType() to return void conf: change virDomainDefMaybeAddVirtioSerialController() to return void conf: change virDomainDefMaybeAddSmartcardController() to return void conf: change virDomainDefAddImplicitControllers() to return void
I'll have a look at these today.
src/bhyve/bhyve_domain.c | 13 ++- src/conf/domain_addr.c | 6 +- src/conf/domain_conf.c | 174 ++++++++++----------------------- src/conf/domain_conf.h | 16 +-- src/hyperv/hyperv_driver.c | 28 ++---- src/libxl/libxl_conf.c | 4 +- src/libxl/libxl_domain.c | 11 +-- src/libxl/libxl_driver.c | 11 +-- src/libxl/xen_common.c | 15 +-- src/libxl/xen_common.h | 2 +- src/libxl/xen_xl.c | 4 +- src/lxc/lxc_driver.c | 6 +- src/qemu/qemu_domain_address.c | 8 +- src/qemu/qemu_driver.c | 14 +-- src/qemu/qemu_postparse.c | 58 ++++------- src/qemu/qemu_process.c | 3 +- src/vbox/vbox_common.c | 13 +-- src/vmx/vmx.c | 12 +-- src/vz/vz_driver.c | 11 +-- src/vz/vz_sdk.c | 14 +-- 20 files changed, 126 insertions(+), 297 deletions(-) -- 2.47.1
Attachment:
signature.asc
Description: PGP signature
