[libvirt] [PATCH] test: Remove possible infinite loop in virnetsockettest

Commit 39015a6f3 modified the test to be more reliable/realistic, but without checking the return status of virEventRunDefaultImpl it's possible that the test could run infinitely. Found by Coverity Signed-off-by: John Ferlan <jferlan@redhat.com> --- tests/virnetsockettest.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index cccb90d0be..490f720fd0 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -243,8 +243,13 @@ testSocketAccept(const void *opaque) &cdata) < 0) goto cleanup; - while (rsock == NULL) - virEventRunDefaultImpl(); + while (rsock == NULL) { + if (virEventRunDefaultImpl() < 0) { + VIR_WARN("Failed to run event loop: %s\n", + virGetLastErrorMessage()); + break; + } + } for (i = 0; i < nlsock; i++) { if (lsock[i] == rsock) { -- 2.17.1

On 09/07/2018 02:26 PM, John Ferlan wrote:
Commit 39015a6f3 modified the test to be more reliable/realistic, but without checking the return status of virEventRunDefaultImpl it's possible that the test could run infinitely.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com> --- tests/virnetsockettest.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index cccb90d0be..490f720fd0 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -243,8 +243,13 @@ testSocketAccept(const void *opaque) &cdata) < 0) goto cleanup;
- while (rsock == NULL) - virEventRunDefaultImpl(); + while (rsock == NULL) { + if (virEventRunDefaultImpl() < 0) { + VIR_WARN("Failed to run event loop: %s\n", + virGetLastErrorMessage());
The libvirt error is dispatched in case of error. Do we need this VIR_WARN() then?
+ break; + } + }
for (i = 0; i < nlsock; i++) { if (lsock[i] == rsock) {
ACK regardless. Michal

On 09/11/2018 07:19 AM, Michal Privoznik wrote:
On 09/07/2018 02:26 PM, John Ferlan wrote:
Commit 39015a6f3 modified the test to be more reliable/realistic, but without checking the return status of virEventRunDefaultImpl it's possible that the test could run infinitely.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com> --- tests/virnetsockettest.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index cccb90d0be..490f720fd0 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -243,8 +243,13 @@ testSocketAccept(const void *opaque) &cdata) < 0) goto cleanup;
- while (rsock == NULL) - virEventRunDefaultImpl(); + while (rsock == NULL) { + if (virEventRunDefaultImpl() < 0) { + VIR_WARN("Failed to run event loop: %s\n", + virGetLastErrorMessage());
The libvirt error is dispatched in case of error. Do we need this VIR_WARN() then?
Probably not, especially since it's only a test... Guess I was being over paranoid. I'll remove before pushing... Tks, John
+ break; + } + }
for (i = 0; i < nlsock; i++) { if (lsock[i] == rsock) {
ACK regardless.
Michal
participants (2)
-
John Ferlan
-
Michal Privoznik