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/ :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
