On 12/20/2010 01:03 AM, Laine Stump wrote: > The functions in iptables.c all return -1 on failure, but all their > callers (which all happen to be in bridge_driver.c) assume that they > are returning an errno, and the logging is done accordingly. This > patch fixes all the error checking and logging to assume < 0 is an > error, and nothing else. > --- > src/network/bridge_driver.c | 183 +++++++++++++++++++++---------------------- > 1 files changed, 91 insertions(+), 92 deletions(-) Do any of the iptables.c functions guarantee that errno is a sane value when returning -1? > - virReportSystemError(err, > - _("failed to add iptables rule to allow forwarding from '%s'"), > - network->def->bridge); > + if (iptablesAddForwardAllowOut(driver->iptables, > + &network->def->ipAddress, > + &network->def->netmask, > + network->def->bridge, > + network->def->forwardDev) < 0) { > + networkReportError(VIR_ERR_SYSTEM_ERROR, > + _("failed to add iptables rule to allow forwarding from '%s'"), > + network->def->bridge); or are we okay with this slightly less-informative message, if we happen to be ignoring a valid errno? Then again, given that the old code was using strerror(-1), which doesn't convey any information, we aren't really losing anything in practice from the old code by not displaying errno, even if iptables guaranteed that errno was useful. Therefore: ACK as-is. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list