[Qemu-devel] [PULL 0/3] Tracing patches

[Qemu-devel] [PULL 0/3] Tracing patches

[Stefan Hajnoczi]

The following changes since commit 6233b4a8c2a32ef6955a921246fa08705bbb3676:

  Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2018-01-26' into staging (2018-01-26 17:29:14 +0000)

are available in the Git repository at:

  git://github.com/stefanha/qemu.git tags/tracing-pull-request

for you to fetch changes up to 24f4d3d3aeabf83445839099d6d66cbb3089c37a:

  tracetool: report error on foo() instead of foo(void) (2018-01-29 10:34:55 +0000)

----------------------------------------------------------------

----------------------------------------------------------------

Stefan Hajnoczi (3):
  tracetool: prefix parse errors with line numbers
  tracetool: clarify that "formats" means "format strings"
  tracetool: report error on foo() instead of foo(void)

 scripts/tracetool/__init__.py | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

-- 
2.14.3

[Qemu-devel] [PULL 1/3] tracetool: prefix parse errors with line numbers

[Stefan Hajnoczi]

Include the file line number in the message that is printed when
trace-events parse errors are raised.

[Use enumerate(fobj, 1) to avoid having to increment a 0-based index
later, as suggested by Eric Blake.
--Stefan]

Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180110202553.31889-2-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool/__init__.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 0670ec17d5..a744e26f91 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -300,13 +300,18 @@ def read_events(fobj):
     """
 
     events = []
-    for line in fobj:
+    for lineno, line in enumerate(fobj, 1):
         if not line.strip():
             continue
         if line.lstrip().startswith('#'):
             continue
 
-        event = Event.build(line)
+        try:
+            event = Event.build(line)
+        except ValueError as e:
+            arg0 = 'Error on line %d: %s' % (lineno, e.args[0])
+            e.args = (arg0,) + e.args[1:]
+            raise
 
         # transform TCG-enabled events
         if "tcg" not in event.properties:
-- 
2.14.3

[Qemu-devel] [PULL 2/3] tracetool: clarify that "formats" means "format strings"

[Stefan Hajnoczi]

The terminology used by tracetool is not consistent with C sprintf or
docs/devel/tracing.txt.  The word "formats" is sometimes used to mean
"format strings".

This patch clarifies comments and error messages that contain this word.

Note that the error message lines are longer than 80 characters but I
have not wrapped them to aid grepping.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180110202553.31889-3-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool/__init__.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index a744e26f91..e3685bd0ca 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -173,7 +173,7 @@ class Event(object):
         props : list of str
             Property names.
         fmt : str, list of str
-            Event printing format (or formats).
+            Event printing format string(s).
         args : Arguments
             Event arguments.
         orig : Event or None
@@ -237,9 +237,9 @@ class Event(object):
         if "tcg-exec" in props:
             raise ValueError("Invalid property 'tcg-exec'")
         if "tcg" not in props and not isinstance(fmt, str):
-            raise ValueError("Only events with 'tcg' property can have two formats")
+            raise ValueError("Only events with 'tcg' property can have two format strings")
         if "tcg" in props and isinstance(fmt, str):
-            raise ValueError("Events with 'tcg' property must have two formats")
+            raise ValueError("Events with 'tcg' property must have two format strings")
 
         event = Event(name, props, fmt, args)
 
@@ -263,7 +263,7 @@ class Event(object):
     _FMT = re.compile("(%[\d\.]*\w+|%.*PRI\S+)")
 
     def formats(self):
-        """List of argument print formats."""
+        """List conversion specifiers in the argument print format string."""
         assert not isinstance(self.fmt, list)
         return self._FMT.findall(self.fmt)
 
-- 
2.14.3

[Qemu-devel] [PULL 3/3] tracetool: report error on foo() instead of foo(void)

[Stefan Hajnoczi]

C functions with no arguments must be declared foo(void) instead of
foo().  The tracetool argument list parser has never accepted an empty
argument list.  This patch adds a clear error message for this error
case.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180110202553.31889-4-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool/__init__.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index e3685bd0ca..1a9733da9a 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -75,6 +75,8 @@ class Arguments:
         res = []
         for arg in arg_str.split(","):
             arg = arg.strip()
+            if not arg:
+                raise ValueError("Empty argument (did you forget to use 'void'?)")
             if arg == 'void':
                 continue
 
-- 
2.14.3

Re: [Qemu-devel] [PULL 0/3] Tracing patches

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180129160740.1195-1-stefanha@redhat.com
Subject: [Qemu-devel] [PULL 0/3] Tracing patches

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   fccfcc6328..0d1442912b  master     -> master
 t [tag update]            patchew/20171130164750.47320-1-vsementsov@virtuozzo.com -> patchew/20171130164750.47320-1-vsementsov@virtuozzo.com
 * [new tag]               patchew/20180129160740.1195-1-stefanha@redhat.com -> patchew/20180129160740.1195-1-stefanha@redhat.com
Switched to a new branch 'test'
8224c195a1 tracetool: report error on foo() instead of foo(void)
511aad4f63 tracetool: clarify that "formats" means "format strings"
cda21c769c tracetool: prefix parse errors with line numbers

=== OUTPUT BEGIN ===
Checking PATCH 1/3: tracetool: prefix parse errors with line numbers...
Checking PATCH 2/3: tracetool: clarify that "formats" means "format strings"...
ERROR: line over 90 characters
#39: FILE: scripts/tracetool/__init__.py:240:
+            raise ValueError("Only events with 'tcg' property can have two format strings")

WARNING: line over 80 characters
#42: FILE: scripts/tracetool/__init__.py:242:
+            raise ValueError("Events with 'tcg' property must have two format strings")

total: 1 errors, 1 warnings, 27 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/3: tracetool: report error on foo() instead of foo(void)...
WARNING: line over 80 characters
#26: FILE: scripts/tracetool/__init__.py:79:
+                raise ValueError("Empty argument (did you forget to use 'void'?)")

total: 0 errors, 1 warnings, 8 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PULL 0/3] Tracing patches

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1413 bytes --]

On Mon, Jan 29, 2018 at 08:14:46AM -0800, no-reply@patchew.org wrote:
> === OUTPUT BEGIN ===
> Checking PATCH 1/3: tracetool: prefix parse errors with line numbers...
> Checking PATCH 2/3: tracetool: clarify that "formats" means "format strings"...
> ERROR: line over 90 characters
> #39: FILE: scripts/tracetool/__init__.py:240:
> +            raise ValueError("Only events with 'tcg' property can have two format strings")
> 
> WARNING: line over 80 characters
> #42: FILE: scripts/tracetool/__init__.py:242:
> +            raise ValueError("Events with 'tcg' property must have two format strings")
> 
> total: 1 errors, 1 warnings, 27 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 3/3: tracetool: report error on foo() instead of foo(void)...
> WARNING: line over 80 characters
> #26: FILE: scripts/tracetool/__init__.py:79:
> +                raise ValueError("Empty argument (did you forget to use 'void'?)")
> 
> total: 0 errors, 1 warnings, 8 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> === OUTPUT END ===

For the record, I didn't wrap these error messages so that grep(1)
works.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PULL 0/3] Tracing patches

[Peter Maydell]

On 29 January 2018 at 16:07, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit 6233b4a8c2a32ef6955a921246fa08705bbb3676:
>
>   Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2018-01-26' into staging (2018-01-26 17:29:14 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/stefanha/qemu.git tags/tracing-pull-request
>
> for you to fetch changes up to 24f4d3d3aeabf83445839099d6d66cbb3089c37a:
>
>   tracetool: report error on foo() instead of foo(void) (2018-01-29 10:34:55 +0000)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------
>
> Stefan Hajnoczi (3):
>   tracetool: prefix parse errors with line numbers
>   tracetool: clarify that "formats" means "format strings"
>   tracetool: report error on foo() instead of foo(void)
>
>  scripts/tracetool/__init__.py | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)

Applied, thanks.

-- PMM