Re: [libvirt] [PATCH v3 4/4] bridge driver: don't masquerade local subnet broadcast/multicast packets
On 09/24/2013 07:23 AM, Laszlo Ersek wrote:
>> +
>> + localMulticast->prefix = 24;
>> + result = virSocketAddrParseIPv4(&localMulticast->addr,
>> + "224.0.0.0");
>> + sa_assert(result != -1);
>
> You must have accidentally left this in. libvirt is a library, so it
> must never assert. In a case where the called function is guaranteed to
> never fail (due to the args passed in), you can enclose it in
> ignore_value():
Libraries must NOT use assert(). But libvirt MAY use sa_assert() -
which exists only as a hint to shut up puny static analyzers and NOT as
a way to abort execution if the constraint is violated (of course, if
the constraint is violated, we still have a bug that needs fixing...).
>
> ignore_value(cirSocketAddrParseIPv4(.......)
Ah. Good to know!
In fact I had searched the HACKING file for "assert", and there were no
hits. So I grepped the source :)
We have uses of assert() in tools/virsh*.c, but only because virsh is an
end-user executable, and not a library. But you are correct that we
must NOT have it within daemon/ or src/. And indeed, ignore_value() is
a nice way to declare that we know our particular call isn't worth
checking for failure.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)