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 :) -- Andrea Bolognani / Red Hat / Virtualization