[libvirt] [PATCH 03/10] tests: qemumonitor: Simplify handling of end of file in full file test

Peter Krempa posted 10 patches 6 years, 11 months ago
[libvirt] [PATCH 03/10] tests: qemumonitor: Simplify handling of end of file in full file test
Posted by Peter Krempa 6 years, 11 months ago
On EOF, the loop can be terminated right away since most of it is
skipped anyways and the handling of the last command is repeated after
the loop.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 tests/qemumonitortestutils.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 62f68ee699..a73272e7b0 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -1416,9 +1416,12 @@ qemuMonitorTestNewFromFileFull(const char *fileName,
     tmp = jsonstr;
     command = tmp;
     while ((tmp = strchr(tmp, '\n'))) {
-        bool eof = !tmp[1];
         line++;

+        /* eof */
+        if (!tmp[1])
+            break;
+
         if (*(tmp + 1) != '\n') {
             *tmp = ' ';
             tmp++;
@@ -1434,21 +1437,16 @@ qemuMonitorTestNewFromFileFull(const char *fileName,
                 response = NULL;
             }

-            if (!eof) {
-                /* Move the @tmp and @singleReply. */
-                tmp += 2;
+            /* Move the @tmp and @singleReply. */
+            tmp += 2;

-                if (!command) {
-                    commandln = line;
-                    command = tmp;
-                } else {
-                    response = tmp;
-                }
+            if (!command) {
+                commandln = line;
+                command = tmp;
+            } else {
+                response = tmp;
             }
         }
-
-        if (eof)
-            break;
     }

     if (command) {
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/10] tests: qemumonitor: Simplify handling of end of file in full file test
Posted by John Ferlan 6 years, 11 months ago

On 06/04/2018 09:58 AM, Peter Krempa wrote:
> On EOF, the loop can be terminated right away since most of it is
> skipped anyways and the handling of the last command is repeated after
> the loop.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  tests/qemumonitortestutils.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 

Would this be more readable with virStringSplit[Count] ?  Every time I
see strchr, that's where I go...

What's here is fine, but figured I'd ask anyway.

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/10] tests: qemumonitor: Simplify handling of end of file in full file test
Posted by Peter Krempa 6 years, 11 months ago
On Thu, Jun 07, 2018 at 21:18:30 -0400, John Ferlan wrote:
> 
> 
> On 06/04/2018 09:58 AM, Peter Krempa wrote:
> > On EOF, the loop can be terminated right away since most of it is
> > skipped anyways and the handling of the last command is repeated after
> > the loop.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  tests/qemumonitortestutils.c | 24 +++++++++++-------------
> >  1 file changed, 11 insertions(+), 13 deletions(-)
> > 
> 
> Would this be more readable with virStringSplit[Count] ?  Every time I
> see strchr, that's where I go...
> 
> What's here is fine, but figured I'd ask anyway.

It's not really possible here as the code is removing newlines from the
code until it hits an empty line. We could use virStringSplitCount to
pick the full reply, but AFAIK we'd still need to remove the single
newlines so that the monitor code does not get confused.

> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> John
> 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list