Trivial fixes and clean ups

A trivial comment typo fix.

Signed-off-by Cleber Rosa <[email protected]>

---
  qemu-img.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

qemu-img.c +0 -0

index b12f4cd19b..a96b76c187 100644

qemu-img.c +0 -0

   * a non-NUL byte.
   *
   * 'pnum' is set to the number of sectors (including and immediately following

Quoted 17 lines · Cleber Rosa

A trivial comment typo fix.

Signed-off-by Cleber Rosa <[email protected]>

---
  qemu-img.c | 2 +-

  1 file changed, 1 insertion(+), 1 deletion(-)

qemu-img.c +0 -1

index b12f4cd19b..a96b76c187 100644
@@ -1085,7 +1085,7 @@ static int64_t find_nonzero(const uint8_t *buf,
 int64_t n)
   }
     /*
- * Returns true iff the first sector pointed to by 'buf' contains at

least

Quoted 2 lines

+ * Returns true if the first sector pointed to by 'buf' contains at least

NACK.  You're not the first person to propose this change.  However, "iff" is an English word (albeit archaic) which is shorthand for "if and only if", which has a distinct logical meaning separate from the weaker "if".  Spelling it out in longhand instead of calling it a typo is probably acceptable, though.

Quoted 18 lines · Cleber Rosa

Quoted 17 lines

A trivial comment typo fix.

Signed-off-by Cleber Rosa <[email protected]>

---
  qemu-img.c | 2 +-

  1 file changed, 1 insertion(+), 1 deletion(-)

qemu-img.c +0 -1

index b12f4cd19b..a96b76c187 100644
@@ -1085,7 +1085,7 @@ static int64_t find_nonzero(const uint8_t *buf,
 int64_t n)
   }
     /*
- * Returns true iff the first sector pointed to by 'buf' contains at

least

Quoted 9 lines

Quoted 2 lines

+ * Returns true if the first sector pointed to by 'buf' contains at least

NACK.  You're not the first person to propose this change.  However, "iff" is an English word (albeit archaic) which is shorthand for "if and only if", which has a distinct logical meaning separate from the weaker "if".  Spelling it out in longhand instead of calling it a typo is probably acceptable, though.

Wow, thanks for the explanation. I agree that spelling it out is a good option, as it'll probably prevent other people from falling into the same "language trap" as I did.

Quoted 4 lines · Cleber Rosa

A trivial comment typo fix.

Signed-off-by Cleber Rosa <[email protected]>

Quoted 9 lines

qemu-img.c +0 -1

@@ -1085,7 +1085,7 @@ static int64_t find_nonzero(const uint8_t *buf,
 int64_t n)
  }
    /*
- * Returns true iff the first sector pointed to by 'buf' contains at

least
+ * Returns true if the first sector pointed to by 'buf' contains at least

NACK. You're not the first person to propose this change. However, "iff" is an English word (albeit archaic) which is shorthand for "if and only if", which has a distinct logical meaning separate from the weaker "if". Spelling it out in longhand instead of calling it a typo is probably acceptable, though.

Point to the right and obvious location for lm32 tests.

Signed-off-by Cleber Rosa <[email protected]>

---
 tests/tcg/README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tests/tcg/README +0 -0

index a5643d33e7..2a58f9a058 100644

Point to the right and obvious location for lm32 tests.

Signed-off-by Cleber Rosa <[email protected]>

---
 tests/tcg/README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tests/tcg/README +0 -0

index a5643d33e7..2a58f9a058 100644

Point to the right and obvious location for lm32 tests.

Signed-off-by Cleber Rosa <[email protected]>

---
 tests/tcg/README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tests/tcg/README +0 -0

index a5643d33e7..2a58f9a058 100644

Quoted 9 lines · Cleber Rosa

Commit 990dc39c made all tests executable at the time, but 218 came in later, and missing those permissions.

Signed-off-by Cleber Rosa <[email protected]>

---
 tests/qemu-iotests/218 | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 tests/qemu-iotests/218

tests/qemu-iotests/218

Quoted 3 lines

tests/qemu-iotests/218 +0 -0

old mode 100644
new mode 100755

We could add a git pre-commit hook grep'ing for "^#!\
*(/usr/bin/env|/bin/(b?a)?sh)" in the 1st line and then checking the file perms.

Quoted 5 lines

We could add a git pre-commit hook grep'ing for "^#!\
*(/usr/bin/env|/bin/(b?a)?sh)" in the 1st line and then checking the file perms.

Good idea. Probably better to send that as a different patch, right?

Quoted 7 lines · Cleber Rosa

Quoted 5 lines

We could add a git pre-commit hook grep'ing for "^#!\
*(/usr/bin/env|/bin/(b?a)?sh)" in the 1st line and then checking the file perms.

Good idea.  Probably better to send that as a different patch, right?

Yes, separate patch. For that matter, "^#!/" is probably a sufficient pattern to catch any file that intends to be run via an interpreter, and therefore wants to be executable, rather than limiting to just env and specific shells.

Commit cce293a2945 moved some functions from common.config to common.rc, but the error messages still reference the old file location.

Signed-off-by Cleber Rosa <[email protected]>

---
 tests/qemu-iotests/common.rc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

tests/qemu-iotests/common.rc +2 -2

index 44bee16a5e..70ca65b49b 100644
@@ -170,7 +170,7 @@ if [ ! -e "$TEST_DIR" ]; then
 fi
 
 if [ ! -d "$TEST_DIR" ]; then
-    echo "common.config: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
+    echo "common.rc: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
     exit 1
 fi
 

@@ -179,7 +179,7 @@ if [ -z "$REMOTE_TEST_DIR" ]; then
 fi
 
 if [ ! -d "$SAMPLE_IMG_DIR" ]; then
-    echo "common.config: Error: \$SAMPLE_IMG_DIR ($SAMPLE_IMG_DIR) is not a directory"
+    echo "common.rc: Error: \$SAMPLE_IMG_DIR ($SAMPLE_IMG_DIR) is not a directory"
     exit 1
 fi
 

Commit cce293a2945 moved some functions from common.config to common.rc, but the error messages still reference the old file location.

Signed-off-by Cleber Rosa <[email protected]>

---
 tests/qemu-iotests/common.rc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

The line immediate following a ".. code::" block is considered to contains arguments to the "code directive". The lack of a new line gives me during at parse time:

testing.rst:63: (ERROR/3) Error in "code" directive:
maximum 1 argument(s) allowed, 3 supplied.

.. code::
make check-unit V=1

testing.rst:120: (ERROR/3) Error in "code" directive:
maximum 1 argument(s) allowed, 3 supplied.

.. code::
make check-qtest V=1

Let's add the missing newlines, both for consistency and to avoid the parsing errors.

Signed-off-by Cleber Rosa <[email protected]>

The line immediate following a ".. code::" block is considered to contains arguments to the "code directive". The lack of a new line gives me during at parse time:

testing.rst:63: (ERROR/3) Error in "code" directive:
maximum 1 argument(s) allowed, 3 supplied.

.. code::
make check-unit V=1

testing.rst:120: (ERROR/3) Error in "code" directive:
maximum 1 argument(s) allowed, 3 supplied.

.. code::
make check-qtest V=1

Let's add the missing newlines, both for consistency and to avoid the parsing errors.

Signed-off-by Cleber Rosa <[email protected]>

---
 docs/devel/testing.rst | 2 ++
 1 file changed, 2 insertions(+)

docs/devel/testing.rst +1 -0

index 727c4019b5..9355ad49f2 100644
@@ -61,6 +61,7 @@ variable (which affects memory reclamation and catches invalid pointers better)
 and gtester options. If necessary, you can run
 
 .. code::
+
   make check-unit V=1
 
 and copy the actual command line which executes the unit test, then run

Signed-off-by Cleber Rosa <[email protected]>

---
 thunk.c | 2 --
 1 file changed, 2 deletions(-)

thunk.c +0 -0

index d5d8645cd4..e351ae53af 100644

Le 04/10/2018 à 18:18, Cleber Rosa a écrit :

Quoted 13 lines

Signed-off-by Cleber Rosa <[email protected]>

---
 thunk.c | 2 --
 1 file changed, 2 deletions(-)

thunk.c +0 -0

index d5d8645cd4..e351ae53af 100644
@@ -21,8 +21,6 @@
 #include "qemu.h"
 #include "exec/user/thunk.h"
 

thunk.c line 25

Quoted 2 lines

thunk.c +0 -2

-//#define DEBUG
-

Generally we use that to enable easily existing debug traces.

In this case, if you remove the "#define" I think you should also replace the "#ifdef DEBUG"s by some trace_XXXX() functions.

Thanks,
Laurent

Quoted 14 lines

Le 04/10/2018 à 18:18, Cleber Rosa a écrit :

Quoted 13 lines

Signed-off-by Cleber Rosa <[email protected]>

---
 thunk.c | 2 --
 1 file changed, 2 deletions(-)

thunk.c +0 -0

index d5d8645cd4..e351ae53af 100644
@@ -21,8 +21,6 @@
 #include "qemu.h"
 #include "exec/user/thunk.h"
 

Quoted 5 lines

thunk.c line 25

Quoted 2 lines

thunk.c +0 -2

-//#define DEBUG
-

Generally we use that to enable easily existing debug traces.

Oh, I wasn't aware that was intentional (I clearly lack some background here).

Quoted 3 lines

In this case, if you remove the "#define" I think you should also replace the "#ifdef DEBUG"s by some trace_XXXX() functions.

Is that desirable? I don't want to disrupt the status quo just because it looked odd to me.

Signed-off-by Cleber Rosa <[email protected]>

---
 scripts/decodetree.py | 2 --
 1 file changed, 2 deletions(-)

scripts/decodetree.py +0 -2

index 277f9a9bba..457cffea90 100755
@@ -149,12 +149,10 @@
 #   trans_addl_i(ctx, &arg_opi, insn)
 #
 
-import io
 import os
 import re
 import sys
 import getopt
-import pdb
 
 insnwidth = 32
 insnmask = 0xffffffff

Signed-off-by Cleber Rosa <[email protected]>

---
 scripts/decodetree.py | 2 --
 1 file changed, 2 deletions(-)

scripts/decodetree.py +0 -0

index 277f9a9bba..457cffea90 100755
@@ -149,12 +149,10 @@
 #   trans_addl_i(ctx, &arg_opi, insn)
 #

Signed-off-by Cleber Rosa <[email protected]>

---
 scripts/decodetree.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/decodetree.py +1 -1

index 457cffea90..37c76b5507 100755
@@ -298,7 +298,7 @@ class Field:
             s = 's'
         else:
             s = ''
-        return str(pos) + ':' + s + str(len)
+        return str(self.pos) + ':' + s + str(self.len)
 
     def str_extract(self):
         if self.sign:

Quoted 6 lines · Cleber Rosa

Signed-off-by Cleber Rosa <[email protected]>

---
  dtc             |  2 +-
  scripts/qemu.py | 65 +++++++++++++++++++++++++++++++------------------

  2 files changed, 42 insertions(+), 25 deletions(-)

dtc line 1

Quoted 7 lines

dtc +1 -1

index 88f18909db..e54388015a 160000
@@ -1 +1 @@
-Subproject commit 88f18909db731a627456f26d779445f84e449536
+Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42

The submodule change was probably unintended.

Signed-off-by Cleber Rosa <[email protected]>

---
 dtc             |  2 +-
 scripts/qemu.py | 65 +++++++++++++++++++++++++++++++------------------
 2 files changed, 42 insertions(+), 25 deletions(-)

dtc +1 -1

index 88f18909db..e54388015a 160000
@@ -1 +1 @@
-Subproject commit 88f18909db731a627456f26d779445f84e449536
+Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42

scripts/qemu.py +0 -0

index f099ce7278..7abe26de69 100644

scripts/qemu.py +0 -1

     def __init__(self, reply):
         try:
             desc = reply["error"]["desc"]

@@ -66,14 +66,15 @@ class MonitorResponseError(qmp.qmp.QMPError):
 
 
 class QEMUMachine(object):
-    '''A QEMU VM

scripts/qemu.py +33 -19

         options = ['fd=%d' % fd,
                    'set=%d' % fdset,
                    'opaque=%s' % opaque]

@@ -168,7 +171,9 @@ class QEMUMachine(object):
 
     @staticmethod
     def _remove_if_exists(path):
-        '''Remove file object at path if it exists'''
+        """
+        Remove file object at path if it exists
+        """
         try:
             os.remove(path)
         except OSError as exception:

@@ -271,7 +276,9 @@ class QEMUMachine(object):
             raise
 
     def _launch(self):
-        '''Launch the VM and establish a QMP connection'''
+        """
+        Launch the VM and establish a QMP connection
+        """
         devnull = open(os.path.devnull, 'rb')
         self._pre_launch()
         self._qemu_full_args = (self._wrapper + [self._binary] +

@@ -284,14 +291,18 @@ class QEMUMachine(object):
         self._post_launch()
 
     def wait(self):
-        '''Wait for the VM to power off'''
+        """
+        Wait for the VM to power off
+        """
         self._popen.wait()
         self._qmp.close()
         self._load_io_log()
         self._post_shutdown()
 
     def shutdown(self):
-        '''Terminate the VM and clean up'''
+        """
+        Terminate the VM and clean up
+        """
         if self.is_running():
             try:
                 self._qmp.cmd('quit')

@@ -315,7 +326,9 @@ class QEMUMachine(object):
         self._launched = False
 
     def qmp(self, cmd, conv_keys=True, **args):
-        '''Invoke a QMP command and return the response dict'''
+        """
+        Invoke a QMP command and return the response dict
+        """
         qmp_args = dict()
         for key, value in args.items():
             if conv_keys:

@@ -326,11 +339,11 @@ class QEMUMachine(object):
         return self._qmp.cmd(cmd, args=qmp_args)
 
     def command(self, cmd, conv_keys=True, **args):
-        '''
+        """
         Invoke a QMP command.
         On success return the response dict.
         On failure raise an exception.
-        '''
+        """
         reply = self.qmp(cmd, conv_keys, **args)
         if reply is None:
             raise qmp.qmp.QMPError("Monitor is closed")

@@ -339,13 +352,17 @@ class QEMUMachine(object):
         return reply["return"]
 
     def get_qmp_event(self, wait=False):
-        '''Poll for one queued QMP events and return it'''
+        """
+        Poll for one queued QMP events and return it
+        """
         if len(self._events) > 0:
             return self._events.pop(0)
         return self._qmp.pull_event(wait=wait)
 
     def get_qmp_events(self, wait=False):
-        '''Poll for queued QMP events and return a list of dicts'''
+        """
+        Poll for queued QMP events and return a list of dicts
+        """
         events = self._qmp.get_events(wait=wait)
         events.extend(self._events)
         del self._events[:]

@@ -353,7 +370,7 @@ class QEMUMachine(object):
         return events
 
     def event_wait(self, name, timeout=60.0, match=None):
-        '''
+        """
         Wait for specified timeout on named event in QMP; optionally filter
         results by match.
 

@@ -361,7 +378,7 @@ class QEMUMachine(object):
         branch processing on match's value None
            {"foo": {"bar": 1}} matches {"foo": None}
            {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
-        '''
+        """
         def event_match(event, match=None):
             if match is None:
                 return True

@@ -394,29 +411,29 @@ class QEMUMachine(object):
         return None
 
     def get_log(self):
-        '''
+        """
         After self.shutdown or failed qemu execution, this returns the output
         of the qemu process.
-        '''
+        """
         return self._iolog
 
     def add_args(self, *args):
-        '''
+        """
         Adds to the list of extra arguments to be given to the QEMU binary
-        '''
+        """
         self._args.extend(args)
 
     def set_machine(self, machine_type):
-        '''
+        """
         Sets the machine type
 
         If set, the machine type will be added to the base arguments
         of the resulting QEMU command line.
-        '''
+        """
         self._machine = machine_type
 
     def set_console(self, device_type=None):
-        '''
+        """
         Sets the device type for a console device
 
         If set, the console device and a backing character device will

@@ -434,7 +451,7 @@ class QEMUMachine(object):
         @param device_type: the device type, such as "isa-serial"
         @raises: QEMUMachineAddDeviceError if the device type is not given
                  and can not be determined.
-        '''
+        """
         if device_type is None:
             if self._machine is None:
                 raise QEMUMachineAddDeviceError("Can not add a console device:"

Quoted 18 lines · Cleber Rosa

Signed-off-by Cleber Rosa <[email protected]>

---
 dtc             |  2 +-
 scripts/qemu.py | 65 +++++++++++++++++++++++++++++++------------------
 2 files changed, 42 insertions(+), 25 deletions(-)

dtc +1 -1

index 88f18909db..e54388015a 160000
@@ -1 +1 @@
-Subproject commit 88f18909db731a627456f26d779445f84e449536
+Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42

scripts/qemu.py +0 -0

index f099ce7278..7abe26de69 100644

scripts/qemu.py class QEMUMachineAddDeviceError(QEMUMachineError): · line 58

Quoted 8 lines

scripts/qemu.py +2 -2

@@ -53,9 +53,9 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
     """
 
 class MonitorResponseError(qmp.qmp.QMPError):
-    '''
+    """
     Represents erroneous QMP monitor reply
-    '''
+    """

This seems obviously correct, as per the Python Dogma Handbook ...

scripts/qemu.py class QEMUMachine(object): · line 141

Quoted 5 lines

def add_fd(self, fd, fdset, opaque, opts=''):

scripts/qemu.py +3 -1

-        '''Pass a file descriptor to the VM'''
+        """
+        Pass a file descriptor to the VM
+        """

However, is it established practice among ne'er-do-wells to format one-line docstrings as three-liners? (And without punctuation to boot -- for shame!)

PEP257 suggests that one-liners are allowed, but doesn't seem to necessitate their usage. Does this kind of change have any kind of benefit?

Signed-off-by Cleber Rosa <[email protected]>

---
 scripts/qemu.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/qemu.py +1 -1

index 7abe26de69..676eb9709a 100644
@@ -88,7 +88,7 @@ class QEMUMachine(object):
         @param name: prefix for socket and log file names (default: qemu-PID)
         @param test_dir: where to create socket and log file
         @param monitor_address: address for QMP monitor
-        @param socket_scm_helper: helper program, required for send_fd_scm()"
+        @param socket_scm_helper: helper program, required for send_fd_scm()
         @note: Qemu process is not started until launch() is used.
         '''
         if args is None:

Signed-off-by Cleber Rosa <[email protected]>

---
 scripts/qemu.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/qemu.py +0 -0

index 7abe26de69..676eb9709a 100644

Just a collection of trivial fixes and clean ups that have been lying around here for some time.