Re: [libvirt] [go PATCH] Add support for virNetworkPort object and APIs
On 18 Jun 2019, at 19:01, Daniel P. Berrangé
<berrange(a)redhat.com> wrote:
On Tue, Jun 18, 2019 at 06:33:01PM +0200, Christophe de Dinechin wrote:
>
>
>> On 18 Jun 2019, at 17:43, Daniel P. Berrangé <berrange(a)redhat.com> wrote:
>>
>> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
>> ---
>> network.go | 80 ++++++++++++++
>> network_port.go | 233 ++++++++++++++++++++++++++++++++++++++++
>> network_port_compat.h | 67 ++++++++++++
>> network_port_wrapper.go | 197 +++++++++++++++++++++++++++++++++
>> network_port_wrapper.h | 79 ++++++++++++++
>> network_wrapper.go | 73 +++++++++++++
>> network_wrapper.h | 23 ++++
>> 7 files changed, 752 insertions(+)
>> create mode 100644 network_port.go
>> create mode 100644 network_port_compat.h
>> create mode 100644 network_port_wrapper.go
>> create mode 100644 network_port_wrapper.h
>> +package libvirt
>> +
>> +/*
>> +#cgo pkg-config: libvirt
>> +#include <assert.h>
>> +#include "network_port_wrapper.h"
>> +
>> +virNetworkPtr
>> +virNetworkPortGetNetworkWrapper(virNetworkPortPtr port,
>> + virErrorPtr err)
>> +{
>> +#if LIBVIR_VERSION_NUMBER < 5005000
>> + assert(0); // Caller should have checked version
>
> What about
>
> assert(!”C function not available in this version - Caller should have checked
version”);
>
> ?
That's certainly possible, but I'm not sure its worth bothering with
since this should never be hit in practice & if it is hit we'll see
the offending code in the crash dump easily enough.
By construction, this case happens for a non-libvirt developer.
The stack trace is useless, nothing in the names there explains
that it’s a version mismatch. So the non-libvirt Go developer will need
to install C source code just to be able to read a comment that could
have been shown in the crash? To me, it looks like like the bother
is much higher on their side than on yours.
Also, given that the sole purpose of these wrappers is to fail if
the underlying C function does not exist, would it make sense to
just not emit the wrapper in that case? That would lead to a link-time
failure that might be even more friendly, insofar as it catches things earlier.
Cheers,
Christophe
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|