Like in the rest of our code we tend to prefer 'goto' and 'cleanup' over 'if else' code structure. Do the same here. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- examples/object-events/event-test.c | 131 +++++++++++++++++++----------------- 1 file changed, 69 insertions(+), 62 deletions(-) diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c index afac100..240caac 100644 --- a/examples/object-events/event-test.c +++ b/examples/object-events/event-test.c @@ -607,6 +607,8 @@ static void stop(int sig) int main(int argc, char **argv) { + int ret = EXIT_FAILURE; + virConnectPtr dconn = NULL; int callback1ret = -1; int callback2ret = -1; int callback3ret = -1; @@ -634,28 +636,27 @@ int main(int argc, char **argv) if (argc > 1 && STREQ(argv[1], "--help")) { usage(argv[0]); - return -1; + goto cleanup; } if (virInitialize() < 0) { fprintf(stderr, "Failed to initialize libvirt"); - return -1; + goto cleanup; } if (virEventRegisterDefaultImpl() < 0) { virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to register event implementation: %s\n", err && err->message ? err->message: "Unknown error"); - return -1; + goto cleanup; } - virConnectPtr dconn = NULL; dconn = virConnectOpenAuth(argc > 1 ? argv[1] : NULL, virConnectAuthPtrDefault, VIR_CONNECT_RO); if (!dconn) { printf("error opening\n"); - return -1; + goto cleanup; } virConnectRegisterCloseCallback(dconn, @@ -760,69 +761,75 @@ int main(int argc, char **argv) VIR_DOMAIN_EVENT_CALLBACK(myDomainEventDeviceAddedCallback), strdup("device added"), myFreeFunc); - if ((callback1ret != -1) && - (callback2ret != -1) && - (callback3ret != -1) && - (callback4ret != -1) && - (callback5ret != -1) && - (callback6ret != -1) && - (callback7ret != -1) && - (callback9ret != -1) && - (callback10ret != -1) && - (callback11ret != -1) && - (callback12ret != -1) && - (callback13ret != -1) && - (callback14ret != -1) && - (callback15ret != -1) && - (callback16ret != -1) && - (callback17ret != -1) && - (callback18ret != -1) && - (callback19ret != -1)) { - if (virConnectSetKeepAlive(dconn, 5, 3) < 0) { + if ((callback1ret == -1) || + (callback2ret == -1) || + (callback3ret == -1) || + (callback4ret == -1) || + (callback5ret == -1) || + (callback6ret == -1) || + (callback7ret == -1) || + (callback9ret == -1) || + (callback10ret == -1) || + (callback11ret == -1) || + (callback12ret == -1) || + (callback13ret == -1) || + (callback14ret == -1) || + (callback15ret == -1) || + (callback16ret == -1) || + (callback17ret == -1) || + (callback18ret == -1) || + (callback19ret == -1)) + goto cleanup; + + if (virConnectSetKeepAlive(dconn, 5, 3) < 0) { + virErrorPtr err = virGetLastError(); + fprintf(stderr, "Failed to start keepalive protocol: %s\n", + err && err->message ? err->message : "Unknown error"); + run = 0; + } + + while (run) { + if (virEventRunDefaultImpl() < 0) { virErrorPtr err = virGetLastError(); - fprintf(stderr, "Failed to start keepalive protocol: %s\n", + fprintf(stderr, "Failed to run event loop: %s\n", err && err->message ? err->message : "Unknown error"); - run = 0; } - - while (run) { - if (virEventRunDefaultImpl() < 0) { - virErrorPtr err = virGetLastError(); - fprintf(stderr, "Failed to run event loop: %s\n", - err && err->message ? err->message : "Unknown error"); - } - } - - VIR_DEBUG("Deregistering event handlers"); - virConnectDomainEventDeregister(dconn, myDomainEventCallback1); - virConnectDomainEventDeregisterAny(dconn, callback2ret); - virConnectDomainEventDeregisterAny(dconn, callback3ret); - virConnectDomainEventDeregisterAny(dconn, callback4ret); - virConnectDomainEventDeregisterAny(dconn, callback5ret); - virConnectDomainEventDeregisterAny(dconn, callback6ret); - virConnectDomainEventDeregisterAny(dconn, callback7ret); - virConnectDomainEventDeregisterAny(dconn, callback9ret); - virConnectDomainEventDeregisterAny(dconn, callback10ret); - virConnectDomainEventDeregisterAny(dconn, callback11ret); - virConnectDomainEventDeregisterAny(dconn, callback12ret); - virConnectDomainEventDeregisterAny(dconn, callback13ret); - virConnectDomainEventDeregisterAny(dconn, callback14ret); - virConnectDomainEventDeregisterAny(dconn, callback15ret); - virConnectNetworkEventDeregisterAny(dconn, callback16ret); - virConnectDomainEventDeregisterAny(dconn, callback17ret); - virConnectDomainEventDeregisterAny(dconn, callback18ret); - virConnectDomainEventDeregisterAny(dconn, callback19ret); - - if (callback8ret != -1) - virConnectDomainEventDeregisterAny(dconn, callback8ret); } + VIR_DEBUG("Deregistering event handlers"); + virConnectDomainEventDeregister(dconn, myDomainEventCallback1); + virConnectDomainEventDeregisterAny(dconn, callback2ret); + virConnectDomainEventDeregisterAny(dconn, callback3ret); + virConnectDomainEventDeregisterAny(dconn, callback4ret); + virConnectDomainEventDeregisterAny(dconn, callback5ret); + virConnectDomainEventDeregisterAny(dconn, callback6ret); + virConnectDomainEventDeregisterAny(dconn, callback7ret); + virConnectDomainEventDeregisterAny(dconn, callback9ret); + virConnectDomainEventDeregisterAny(dconn, callback10ret); + virConnectDomainEventDeregisterAny(dconn, callback11ret); + virConnectDomainEventDeregisterAny(dconn, callback12ret); + virConnectDomainEventDeregisterAny(dconn, callback13ret); + virConnectDomainEventDeregisterAny(dconn, callback14ret); + virConnectDomainEventDeregisterAny(dconn, callback15ret); + virConnectNetworkEventDeregisterAny(dconn, callback16ret); + virConnectDomainEventDeregisterAny(dconn, callback17ret); + virConnectDomainEventDeregisterAny(dconn, callback18ret); + virConnectDomainEventDeregisterAny(dconn, callback19ret); + + if (callback8ret != -1) + virConnectDomainEventDeregisterAny(dconn, callback8ret); + virConnectUnregisterCloseCallback(dconn, connectClose); + ret = EXIT_SUCCESS; - VIR_DEBUG("Closing connection"); - if (dconn && virConnectClose(dconn) < 0) - printf("error closing\n"); - printf("done\n"); - return 0; + cleanup: + if (dconn) { + VIR_DEBUG("Closing connection"); + if (virConnectClose(dconn) < 0) + printf("error closing\n"); + printf("done\n"); + } + + return ret; } -- 2.4.10

The adminDispatchConnectListServers() function is generated by our great perl script. However, it has a tiny flaw: if adminConnectListServers() it calls fails, the control jumps onto cleanup label where we try to free any list of servers built so far. However, in the loop @i is unsigned (size_t) while @nresults is signed (int). Currently, it does no harm because of the check for @result being non-NULL. But if that ever changes in the future, this bug will be hard to chase. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/rpc/gendispatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index fc8423a..624049b 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -917,7 +917,7 @@ elsif ($mode eq "server") { print " int rv = -1;\n"; if ($modern_ret_as_list) { - print " size_t i;\n"; + print " ssize_t i;\n"; } foreach my $var (@vars_list) { -- 2.4.10