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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org