Re: [libvirt] [PATCH 4/8] conf: Allow NULL for virDomainDeviceInfoCallback

On Tue, 2019-05-21 at 16:28 +0200, Ján Tomko wrote: Well of course you're avoiding most of the churn I was mentioning: you're adding a new function instead of renaming the existing one! That's cheating ;) I don't much like the idea of adding a separate function that does almost the same thing but not quite, because that will ultimately result in the same situation we have now: someone will add a new callback and (reasonably) expect it to be called for all devices, but that won't happen because the original code uses the DeviceInfo variant instead of the Device one. That's unnecessary friction. Not to mention that your new function also happens to iterate over all consoles, which the existing variant doesn't. That's an extra little inconsistency that we really don't need to introduce. So I still prefer my approach. As said earlier, I'm also not entirely convinced keeping the existing function name is a good idea, so I'll happily rename it at the same time as I'm updating and documenting its contract. Especially once that's done, I don't see any problem with passing the callback a pointer that might be NULL and expecting the user to check before using it, as that's par for the course when using the C language. -- Andrea Bolognani / Red Hat / Virtualization