[Qemu-devel] [PATCH v3 0/4] trace-events: print 0x before hex numbers

Vladimir Sementsov-Ogievskiy posted 4 patches 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170731160135.12101-1-vsementsov@virtuozzo.com
Test FreeBSD passed
Test checkpatch failed
Test docker passed
Test s390x passed
CODING_STYLE              |  35 +++++++++
accel/tcg/trace-events    |   2 +-
audio/trace-events        |   4 +-
block/trace-events        |  28 ++++----
hw/audio/trace-events     |   4 +-
hw/char/trace-events      |  12 ++--
hw/display/trace-events   |  14 ++--
hw/dma/trace-events       |  20 +++---
hw/i386/xen/trace-events  |  26 +++----
hw/input/trace-events     |   6 +-
hw/intc/trace-events      | 176 +++++++++++++++++++++++-----------------------
hw/isa/trace-events       |   4 +-
hw/misc/trace-events      |  78 ++++++++++----------
hw/net/trace-events       |  52 +++++++-------
hw/nvram/trace-events     |   2 +-
hw/pci/trace-events       |   4 +-
hw/ppc/trace-events       |  64 ++++++++---------
hw/s390x/trace-events     |  20 +++---
hw/scsi/trace-events      | 118 +++++++++++++++----------------
hw/sd/trace-events        |   4 +-
hw/timer/trace-events     |  20 +++---
hw/usb/trace-events       |  56 +++++++--------
hw/vfio/trace-events      |  44 ++++++------
hw/virtio/trace-events    |   6 +-
hw/xen/trace-events       |   8 +--
linux-user/trace-events   |  10 +--
migration/trace-events    |  36 +++++-----
nbd/trace-events          |  18 ++---
net/trace-events          |   4 +-
scripts/checkpatch.pl     |  19 +++++
target/arm/trace-events   |  10 +--
target/s390x/trace-events |   2 +-
target/sparc/trace-events |  30 ++++----
trace-events              |  20 +++---
34 files changed, 505 insertions(+), 451 deletions(-)
[Qemu-devel] [PATCH v3 0/4] trace-events: print 0x before hex numbers
Posted by Vladimir Sementsov-Ogievskiy 6 years, 8 months ago
Hi all!

It is hard to read logs, when there are hex and dec numbers in one line, when
hex number doesn't contain any letters and don't have '0x' prefix.

So, here is a complete solution for the problem:

- add information into CODING_STYLE
- add a check into checkpatch.pl
- fix current state

The new rule for the style is:

Hex numbers should be prefixed by '0x', except groups of numbers,
separated by symbols ' ', '.', ':', '/', however '0x' can be used for
numbers in such groups too. Flag '#' in number format is not allowed.

Note: checkpatch fails on checkpatch change (03) due to long lines.
It is because checkpatch.pl is indented by tabs and when it checks
for long lines it consider tabs as 8 spaces. Looks like nobody cares,
so do I. I see two ways here:
 -  s/\t/    /g
 -  make exclusion in checkpatch.pl for checkpatch.pl to consider tabs
    as 4 spaces, not 8.  However I don't want to fix it in the context
    of these series.

v3: 01 - rewording
    02 - adjust commit message - add two printf flags "'I" into a check
         add Stefan's and Eric's r-bs
    03 - fix commit message s/Accordingly/According/
         add Stefan's r-b
    04 - add Stefan's r-b
         add Cornelia's a-b
v2: almost everything (style, checkpatch, excluding number groups)
V1: was a draft of the idea using two sed commands.

Vladimir Sementsov-Ogievskiy (4):
  coding_style: add point about 0x in trace-events
  trace-events: fix code style: %# -> 0x%
  checkpatch: check trace-events code style
  trace-events: fix code style: print 0x before hex numbers

 CODING_STYLE              |  35 +++++++++
 accel/tcg/trace-events    |   2 +-
 audio/trace-events        |   4 +-
 block/trace-events        |  28 ++++----
 hw/audio/trace-events     |   4 +-
 hw/char/trace-events      |  12 ++--
 hw/display/trace-events   |  14 ++--
 hw/dma/trace-events       |  20 +++---
 hw/i386/xen/trace-events  |  26 +++----
 hw/input/trace-events     |   6 +-
 hw/intc/trace-events      | 176 +++++++++++++++++++++++-----------------------
 hw/isa/trace-events       |   4 +-
 hw/misc/trace-events      |  78 ++++++++++----------
 hw/net/trace-events       |  52 +++++++-------
 hw/nvram/trace-events     |   2 +-
 hw/pci/trace-events       |   4 +-
 hw/ppc/trace-events       |  64 ++++++++---------
 hw/s390x/trace-events     |  20 +++---
 hw/scsi/trace-events      | 118 +++++++++++++++----------------
 hw/sd/trace-events        |   4 +-
 hw/timer/trace-events     |  20 +++---
 hw/usb/trace-events       |  56 +++++++--------
 hw/vfio/trace-events      |  44 ++++++------
 hw/virtio/trace-events    |   6 +-
 hw/xen/trace-events       |   8 +--
 linux-user/trace-events   |  10 +--
 migration/trace-events    |  36 +++++-----
 nbd/trace-events          |  18 ++---
 net/trace-events          |   4 +-
 scripts/checkpatch.pl     |  19 +++++
 target/arm/trace-events   |  10 +--
 target/s390x/trace-events |   2 +-
 target/sparc/trace-events |  30 ++++----
 trace-events              |  20 +++---
 34 files changed, 505 insertions(+), 451 deletions(-)

-- 
2.11.1


Re: [Qemu-devel] [PATCH v3 0/4] trace-events: print 0x before hex numbers
Posted by no-reply@patchew.org 6 years, 8 months ago
Hi,

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

Subject: [Qemu-devel] [PATCH v3 0/4] trace-events: print 0x before hex numbers
Message-id: 20170731160135.12101-1-vsementsov@virtuozzo.com
Type: series

=== 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
Switched to a new branch 'test'
d9245be7be trace-events: fix code style: print 0x before hex numbers
4c4e26430d checkpatch: check trace-events code style
811ef299fd trace-events: fix code style: %# -> 0x%
b908ad1263 coding_style: add point about 0x in trace-events

=== OUTPUT BEGIN ===
Checking PATCH 1/4: coding_style: add point about 0x in trace-events...
Checking PATCH 2/4: trace-events: fix code style: %# -> 0x%...
Checking PATCH 3/4: checkpatch: check trace-events code style...
WARNING: line over 80 characters
#26: FILE: scripts/checkpatch.pl:1343:
+				ERROR("Don't use '#' flag of printf format ('%#') in " .

ERROR: line over 90 characters
#27: FILE: scripts/checkpatch.pl:1344:
+				      "trace-events, use '0x' prefix instead\n" . $herecurr);

ERROR: line over 90 characters
#30: FILE: scripts/checkpatch.pl:1347:
+					qr/%[-+ *.0-9]*([hljztL]|ll|hh)?(x|X|"\s*PRI[xX][^"]*"?)/;

ERROR: line over 90 characters
#32: FILE: scripts/checkpatch.pl:1349:
+				# don't consider groups splitted by [.:/ ], like 2A.20:12ab

WARNING: line over 80 characters
#33: FILE: scripts/checkpatch.pl:1350:
+				my $tmpline = $rawline =~ s/($hex[.:\/ ])+$hex//gr;

WARNING: line over 80 characters
#36: FILE: scripts/checkpatch.pl:1353:
+					ERROR("Hex numbers must be prefixed with '0x'\n" .

total: 3 errors, 3 warnings, 25 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 4/4: trace-events: fix code style: print 0x before hex numbers...
=== 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] [PATCH v3 0/4] trace-events: print 0x before hex numbers
Posted by Stefan Hajnoczi 6 years, 7 months ago
On Mon, Jul 31, 2017 at 07:01:31PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> It is hard to read logs, when there are hex and dec numbers in one line, when
> hex number doesn't contain any letters and don't have '0x' prefix.
> 
> So, here is a complete solution for the problem:
> 
> - add information into CODING_STYLE
> - add a check into checkpatch.pl
> - fix current state
> 
> The new rule for the style is:
> 
> Hex numbers should be prefixed by '0x', except groups of numbers,
> separated by symbols ' ', '.', ':', '/', however '0x' can be used for
> numbers in such groups too. Flag '#' in number format is not allowed.
> 
> Note: checkpatch fails on checkpatch change (03) due to long lines.
> It is because checkpatch.pl is indented by tabs and when it checks
> for long lines it consider tabs as 8 spaces. Looks like nobody cares,
> so do I. I see two ways here:
>  -  s/\t/    /g
>  -  make exclusion in checkpatch.pl for checkpatch.pl to consider tabs
>     as 4 spaces, not 8.  However I don't want to fix it in the context
>     of these series.
> 
> v3: 01 - rewording
>     02 - adjust commit message - add two printf flags "'I" into a check
>          add Stefan's and Eric's r-bs
>     03 - fix commit message s/Accordingly/According/
>          add Stefan's r-b
>     04 - add Stefan's r-b
>          add Cornelia's a-b
> v2: almost everything (style, checkpatch, excluding number groups)
> V1: was a draft of the idea using two sed commands.
> 
> Vladimir Sementsov-Ogievskiy (4):
>   coding_style: add point about 0x in trace-events
>   trace-events: fix code style: %# -> 0x%
>   checkpatch: check trace-events code style
>   trace-events: fix code style: print 0x before hex numbers
> 
>  CODING_STYLE              |  35 +++++++++
>  accel/tcg/trace-events    |   2 +-
>  audio/trace-events        |   4 +-
>  block/trace-events        |  28 ++++----
>  hw/audio/trace-events     |   4 +-
>  hw/char/trace-events      |  12 ++--
>  hw/display/trace-events   |  14 ++--
>  hw/dma/trace-events       |  20 +++---
>  hw/i386/xen/trace-events  |  26 +++----
>  hw/input/trace-events     |   6 +-
>  hw/intc/trace-events      | 176 +++++++++++++++++++++++-----------------------
>  hw/isa/trace-events       |   4 +-
>  hw/misc/trace-events      |  78 ++++++++++----------
>  hw/net/trace-events       |  52 +++++++-------
>  hw/nvram/trace-events     |   2 +-
>  hw/pci/trace-events       |   4 +-
>  hw/ppc/trace-events       |  64 ++++++++---------
>  hw/s390x/trace-events     |  20 +++---
>  hw/scsi/trace-events      | 118 +++++++++++++++----------------
>  hw/sd/trace-events        |   4 +-
>  hw/timer/trace-events     |  20 +++---
>  hw/usb/trace-events       |  56 +++++++--------
>  hw/vfio/trace-events      |  44 ++++++------
>  hw/virtio/trace-events    |   6 +-
>  hw/xen/trace-events       |   8 +--
>  linux-user/trace-events   |  10 +--
>  migration/trace-events    |  36 +++++-----
>  nbd/trace-events          |  18 ++---
>  net/trace-events          |   4 +-
>  scripts/checkpatch.pl     |  19 +++++
>  target/arm/trace-events   |  10 +--
>  target/s390x/trace-events |   2 +-
>  target/sparc/trace-events |  30 ++++----
>  trace-events              |  20 +++---
>  34 files changed, 505 insertions(+), 451 deletions(-)
> 
> -- 
> 2.11.1
> 

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan