[libvirt] [PATCH 1/2] util: Turn virFirewallAddRule() into a macro

[Daniel P. Berrange]

On Tue, Jan 03, 2017 at 02:17:38PM +0100, Andrea Bolognani wrote:
> On Tue, 2017-01-03 at 13:46 +0100, Martin Kletzander wrote:
> > > To solve the issue, turn virFirewallAddRule() from a very thin
> > > wrapper around virFirewallAddRuleFullV() to a macro that expands
> > > to a call to virFirewallAddRuleFull() - itself a very thin wrapper
> > > around the aforementioned virFirewallAddRuleFullV() - with no loss
> > > of functionality or type safety.
> > > ---
> > > This only seems to be required on very specific combinations
> > > of Clang and host OS, eg. I need it on Clang 3.9 / Fedora
> > > rawhide but not on Clang 3.8 or 4.0 / Debian sid.
> > 
> > We sent various patches for this (me, Jan and maybe other people as
> > well).  I never realized it's not a problem with different versions of
> > clang.
> 
> Oh, must have missed the previous attempts to fix this.
> 
> > I would say it's not a problem for us to solve it in this case,
> > however, as I wrote in my solution, it works, but it's undefined from
> > the specification point of view.  Can it work just because
> > virFirewallAddRule() gets optimized into inline function?  it shouldn't
> > be, though...  I don't know.
> 
> I lean towards merging this or a comparable solution. It's
> true that we aren't currently hitting this on our main
> targets, but relying on undefined behavior is definitely
> something we want to avoid, plus I don't see any real
> drawbacks in changing this to a macro.

Agreed, I think this patch is fine - the original code could easily
have been done this way even ignoring the clang issue.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|