Re: [libvirt] [PATCH v2 3/7] util: new virFirewallD APIs + docs

On 2/1/19 8:17 AM, Daniel P. Berrangé wrote: > On Thu, Jan 31, 2019 at 08:24:54PM -0500, Laine Stump wrote: > > +int > > +virFirewallDGetBackend(void) > > +{ > > + DBusConnection *sysbus = virDBusGetSystemBus(); > > + DBusMessage *reply = NULL; > > + virError error; > > + VIR_AUTOFREE(char *) backendStr = NULL; > > + int backend = -1; > > + > > + if (!sysbus) > > + return -1; > > + > > + memset(&error, 0, sizeof(error)); > > + > > + if (virDBusCallMethod(sysbus, > > + &reply, > > + &error, > > + VIR_FIREWALL_FIREWALLD_SERVICE, > > + "/org/fedoraproject/FirewallD1/config", > > + "org.freedesktop.DBus.Properties", > > + "Get", > > + "ss", > > + "org.fedoraproject.FirewallD1.config", > > + "FirewallBackend") < 0) > > + goto cleanup; > > + > > + if (error.level == VIR_ERR_ERROR) { > > + /* we don't want to log any error in the case that > > + * FirewallBackend isn't implemented in this firewalld, since > > + * that just means that it is an old version, and only has an > > + * iptables backend. > > + */ > > + VIR_DEBUG("Failed to get FirewallBackend setting, assuming 'iptables'"); > > + backend = VIR_FIREWALLD_BACKEND_IPTABLES; > > + goto cleanup; > > + } > Surely we need an '} else {' case here to propagate 'error' > as fatal. The point is to ignore all errors. If error.level != VIR_ERR_ERROR, then there is no error to propagate (and if it is then we already ignored it). Am I missing something? (Very likely, since I can count on one hand the number of times I've had to directly interact with an error object.)