Re: [Qemu-devel] [PATCH v3 00/18] Refactor trace to allow modular build

[Daniel P. Berrange]

On Mon, Sep 19, 2016 at 08:54:50AM -0700, address@hidden wrote:
> Hi,
> 
> Your series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Subject: [Qemu-devel] [PATCH v3 00/18] Refactor trace to allow modular build
> Message-id: address@hidden
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> 
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
> 
> # Useful git options
> 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 show --no-patch --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
>  * [new tag]         patchew/address@hidden -> patchew/address@hidden
> Switched to a new branch 'test'
> 439b7d5 trace: introduce a formal group name for trace events
> 97b7a5f trace: pass trace-events to tracetool as a positional param
> 46f2527 trace: push reading of events up a level to tracetool main
> 3f822f1 trace: rename _read_events to read_events
> 20a8ba2 trace: get rid of generated-events.h/generated-events.c
> f6a861d trace: dynamically allocate event IDs at runtime
> 04e85e1 trace: dynamically allocate trace_dstate in CPUState
> 5ed1755 trace: provide mechanism for registering trace events
> 3472cce trace: don't abort qemu if ftrace can't be initialized
> e1d4a86 trace: emit name <-> ID mapping in simpletrace header
> 2270adf trace: remove the TraceEventID and TraceEventVCPUID enums
> 604483a trace: give each trace event a named TraceEvent struct
> 731385d trace: break circular dependancy in event-internal.h
> 34a7e6d trace: remove duplicate control.h includes in generated-tracers.h
> 6af9de1 trace: remove global 'uint16 dstate[]' array
> 5c47d07 trace: remove some now unused functions
> a222901 trace: convert code to use event iterators
> 6d86092 trace: add trace event iterator APIs
> 
> === OUTPUT BEGIN ===
> Checking PATCH 1/18: trace: add trace event iterator APIs...
> Checking PATCH 2/18: trace: convert code to use event iterators...
> Checking PATCH 3/18: trace: remove some now unused functions...
> Checking PATCH 4/18: trace: remove global 'uint16 dstate[]' array...
> WARNING: line over 80 characters
> #262: FILE: trace/event-internal.h:24:
> + * Interpretation of @dstate depends on wether the event has the 'vcpu' 
> property:
> 
> total: 0 errors, 1 warnings, 211 lines checked
> 
> Your patch has style problems, please review.  If any of these errors

[snip many more warnings]

The moral of this story is to not trust code you wrote late on a friday
afternoon :-)

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|