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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux