[libvirt] [PATCH] network_conf.c: correct the value of the 'result' variable

The result variable in virNetworkDNSDefFormat() function should be initialized to -1 at first,only in this way can we use it properly. Signed-off-by: Hongwei Bi <hwbi2008@gmail.com> --- src/conf/network_conf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 447eca4..8ab4e96 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2295,7 +2295,7 @@ static int virNetworkDNSDefFormat(virBufferPtr buf, const virNetworkDNSDef *def) { - int result = 0; + int result = -1; size_t i, j; if (!(def->forwardPlainNames || def->forwarders || def->nhosts || @@ -2363,6 +2363,8 @@ virNetworkDNSDefFormat(virBufferPtr buf, } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</dns>\n"); + + result = 0; out: return result; } -- 1.8.1.2

On 10/31/2013 06:20 PM, Hongwei Bi wrote:
The result variable in virNetworkDNSDefFormat() function should be initialized to -1 at first,only in this way can we use it properly.
Signed-off-by: Hongwei Bi <hwbi2008@gmail.com> --- src/conf/network_conf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
NACK. After applying your patch, 'make check' fails: TEST: networkxml2xmltest !!!!!!!.....!.!!!!!!!!!! 24 FAIL FAIL: networkxml2xmlupdatetest ============================== TEST: networkxml2xmlupdatetest ...!..!!!.!!!....!.!.!....!..!.........! 40 FAIL Please don't submit patches that fail 'make check' or 'make syntax-check'; it is a drain on reviewer resources. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 10/31/2013 07:18 PM, Eric Blake wrote:
On 10/31/2013 06:20 PM, Hongwei Bi wrote:
The result variable in virNetworkDNSDefFormat() function should be initialized to -1 at first,only in this way can we use it properly.
Signed-off-by: Hongwei Bi <hwbi2008@gmail.com> --- src/conf/network_conf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) NACK. After applying your patch, 'make check' fails:
Beyond that, a quick reading of the function shows that 1) all conditions that lead to "goto out" are actually success conditions, and 2) no error checking is done in the function (it's all just using virBuffer calls, and those errors are checked at the end when the buffer is transferred into a char*). Although a great majority of the functions in libvirt use the convention of initializing the return value to -1 then setting it to 0 if we reach the end, this is not universal. As it stands now, this function may not follow the libvirt convention, but it is correct.
participants (3)
-
Eric Blake
-
Hongwei Bi
-
Laine Stump