On Thu, Feb 15, 2024 at 05:46:28 -0800, Andrea Bolognani wrote: > 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 :) You can use: Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx> _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
