Re: [PATCH v2 04/17] qemu: Improve qemuDomainDefaultSCSIControllerModel()

On Thu, Feb 15, 2024 at 01:57:27PM +0100, Peter Krempa wrote: > On Wed, Feb 14, 2024 at 18:11:11 +0100, Andrea Bolognani wrote: > > In order to have a nice API while still keeping the > > implementation short and readable, a small wrapper is required. > > Well arguably the wrapper is not needed unless you really want to use: > > return VIR_DOMAIN_CONTROLLER_MODEL_SCS*; > > rather than: > > *model = VIR_DOMAIN_CONTROLLER_MODEL_SCS*; > return 0; I really do want to be able to use the former style. It doesn't make as big an impact here, but for the USB part the difference is quite stark. > So now this always returns 0 -> success, thus the return value is > pointless. Further up the next patch which modifies the documentation > still keeps the claim. Additionally the description will read: > > Not being able to come up with a value is NOT considered a failure. > > which will mean that this function will never fail. > > IMO it would be sufficient to just declare: > > virDomainControllerModelSCSI > qemuDomainDefaultSCSIControllerModel(const virDomainDef *def, > const virDomainControllerDef *cont, > ... > > and just document the same claims. Thus if _DEFAULT is returned it's up > to the caller to decide. > > Fabricating a return value doesn't seem to make much sense for me and > once you only return success you might as well as return the value > directly. It'd simplify the checker conditions. You're right, I can absolutely get rid of the wrapper. Failure states that needed to be reported actually existed at some point, but those went away as I reworked the code, and I never went back to reconsider the API. Thanks a lot for the suggestion, and consider it done :)