mbox series

[v2,00/60] json: Fixes, error reporting improvements, cleanups

Message ID 20180817150559.16243-1-armbru@redhat.com
Headers show
Series json: Fixes, error reporting improvements, cleanups | expand

Message

Markus Armbruster Aug. 17, 2018, 3:04 p.m. UTC
JSON is such a simple language, so writing a parser should be easy,
shouldn't it?  Well, the evidence is in, and it's a lot of patches.
Summary of fixes:

* Reject ASCII control characters in strings as RFC 7159 specifies

* Reject all invalid UTF-8 sequences, not just some

* Reject invalid \uXXXX escapes

* Implement \uXXXX surrogate pairs as specified by RFC 7159

* Don't ignore \u0000 silently, map it to \xC0\80 (modified UTF-8)

* qobject_from_json() is ridicilously broken for input containing more
  than one value, fix

* Don't ignore trailing unterminated structures

* Less cavalierly cruel error reporting

Topped off with tests and cleanups.

If you're into this kind of disaster relief, commit c7a3f25200c
"qapi.py: Restructure lexer and parser" was even funnier.

This v2 is unlikely to be final: I added three more patches, and
addressed a lot of review comments.  I should also update references
to RFC 7159 to RFC 8259.  But right now this needs to get out for
another round of review.

v2:
* Rebased
* PATCH 01,11-14,16-18,20,22-23,29-36,41,43,45-50,53-55 otherwise
  unchanged
* PATCH 57-60 are new
* R-bys kept unless noted otherwise
* PATCH 02
  - Cover unrecognized keyword [Eric]
* PATCH 03
  - Cover \r [Eric]
* PATCH 04-05
  - Comments touched up [Eric]
* PATCH 06
  - Use qmp_fd_send_raw() just for "\xff" [Eric]
* PATCH 07
  - Plug memory leak [Eric]
* PATCH 08
  - Delay adding coverage for \' until PATCH 09
* PATCH 09
  - Cover \\\0
  - Drop duplicated test case (editing accident) [Eric]
  - Improve surrogate coverage
* PATCH 10
  - Don't lose test coverage for \" and \'
  - R-by dropped
* PATCH 15,27,38-39
  - Cover unkown interpolation specification
  - Cover attempt to interpolate into JSON string
  - R-by of PATCH 15 dropped
* PATCH 19
  - Tweak loop control once more
  - R-by dropped
* PATCH 21,26
  - Update for tweak to PATCH 19
  - I might still drop redundant masking [Eric]
* PATCH 24
  - Commit message improved
* PATCH 25
  - Comment improvement [Eric]
  - Commit message tweaked
* PATCH 28
  - Fix error message to show both halves of an invalid surrogate pair
    [Eric]
  - Fix unpaired leading surrogate followed by \u escape [Paolo]
* PATCH 36
  - I might still rename JSON_INTERPOL & friends [Eric]
* PATCH 37
  - Document lexing interpolations is now optional [Eric]
  - Move deletion of a redundant assignment from PATCH 51 [Eric]
* PATCH 37,42,51-52
  - De-duplicate state transitions common to IN_START and
    IN_START_INTERPOL [Eric]
* PATCH 38
  - Commit message tweaked
* PATCH 39
  - More legible commit message [Eric]
  - Comment fix [Eric]
* PATCH 40
  - Commit message typo [Eric]
* PATCH 44
  - Commit message tab damage [Eric]
* PATCH 56
  - More on QGA synchronization [Eric]
  - I might still move this earlier in the series

Marc-André Lureau (2):
  json: remove useless return value from lexer/parser
  json-parser: simplify and avoid JSONParserContext allocation

Markus Armbruster (58):
  check-qjson: Cover multiple JSON objects in same string
  check-qjson: Cover blank and lexically erroneous input
  check-qjson: Cover whitespace more thoroughly
  qmp-cmd-test: Split off qmp-test
  qmp-test: Cover syntax and lexical errors
  test-qga: Clean up how we test QGA synchronization
  check-qjson: Cover escaped characters more thoroughly, part 1
  check-qjson: Streamline escaped_string()'s test strings
  check-qjson: Cover escaped characters more thoroughly, part 2
  check-qjson: Consolidate partly redundant string tests
  check-qjson: Cover UTF-8 in single quoted strings
  check-qjson: Simplify utf8_string()
  check-qjson: Fix utf8_string() to test all invalid sequences
  check-qjson qmp-test: Cover control characters more thoroughly
  check-qjson: Cover interpolation more thoroughly
  json: Fix lexer to include the bad character in JSON_ERROR token
  json: Reject unescaped control characters
  json: Revamp lexer documentation
  json: Tighten and simplify qstring_from_escaped_str()'s loop
  check-qjson: Document we expect invalid UTF-8 to be rejected
  json: Reject invalid UTF-8 sequences
  json: Report first rather than last parse error
  json: Leave rejecting invalid UTF-8 to parser
  json: Accept overlong \xC0\x80 as U+0000 ("modified UTF-8")
  json: Leave rejecting invalid escape sequences to parser
  json: Simplify parse_string()
  json: Reject invalid \uXXXX, fix \u0000
  json: Fix \uXXXX for surrogate pairs
  check-qjson: Fix and enable utf8_string()'s disabled part
  json: Have lexer call streamer directly
  json: Redesign the callback to consume JSON values
  json: Don't pass null @tokens to json_parser_parse()
  json: Don't create JSON_ERROR tokens that won't be used
  json: Rename token JSON_ESCAPE & friends to JSON_INTERPOL
  json: Treat unwanted interpolation as lexical error
  json: Pass lexical errors and limit violations to callback
  json: Leave rejecting invalid interpolation to parser
  json: Replace %I64d, %I64u by %PRId64, %PRIu64
  json: Nicer recovery from invalid leading zero
  json: Improve names of lexer states related to numbers
  qjson: Fix qobject_from_json() & friends for multiple values
  json: Fix latent parser aborts at end of input
  json: Fix streamer not to ignore trailing unterminated structures
  json: Assert json_parser_parse() consumes all tokens on success
  qjson: Have qobject_from_json() & friends reject empty and blank
  json: Enforce token count and size limits more tightly
  json: Streamline json_message_process_token()
  json: Unbox tokens queue in JSONMessageParser
  json: Eliminate lexer state IN_ERROR and pseudo-token JSON_MIN
  json: Eliminate lexer state IN_WHITESPACE, pseudo-token JSON_SKIP
  json: Make JSONToken opaque outside json-parser.c
  qobject: Drop superfluous includes of qemu-common.h
  json: Clean up headers
  docs/interop/qmp-spec: How to force known good parser state
  tests/drive_del-test: Fix harmless JSON interpolation bug
  json: Keep interpolation state in JSONParserContext
  json: Improve safety of qobject_from_jsonf_nofail() & friends
  json: Support %% in JSON strings when interpolating

 MAINTAINERS                      |    1 +
 block.c                          |    5 -
 docs/interop/qmp-spec.txt        |   42 +-
 include/qapi/qmp/json-lexer.h    |   56 --
 include/qapi/qmp/json-parser.h   |   36 +-
 include/qapi/qmp/json-streamer.h |   46 --
 include/qapi/qmp/qerror.h        |    3 -
 include/qemu/unicode.h           |    1 +
 monitor.c                        |   21 +-
 qapi/qmp-dispatch.c              |    1 -
 qapi/qobject-input-visitor.c     |    5 -
 qga/main.c                       |   15 +-
 qobject/json-lexer.c             |  354 +++++-----
 qobject/json-parser-int.h        |   51 ++
 qobject/json-parser.c            |  377 +++++------
 qobject/json-streamer.c          |  126 ++--
 qobject/qbool.c                  |    1 -
 qobject/qjson.c                  |   31 +-
 qobject/qlist.c                  |    1 -
 qobject/qnull.c                  |    1 -
 qobject/qnum.c                   |    1 -
 qobject/qobject.c                |    1 -
 qobject/qstring.c                |    1 -
 tests/Makefile.include           |    3 +
 tests/check-qjson.c              | 1058 ++++++++++++++++--------------
 tests/drive_del-test.c           |    8 +-
 tests/libqtest.c                 |   57 +-
 tests/libqtest.h                 |   13 +
 tests/qmp-cmd-test.c             |  213 ++++++
 tests/qmp-test.c                 |  252 ++-----
 tests/test-qga.c                 |    3 +-
 util/unicode.c                   |   69 +-
 32 files changed, 1495 insertions(+), 1358 deletions(-)
 delete mode 100644 include/qapi/qmp/json-lexer.h
 delete mode 100644 include/qapi/qmp/json-streamer.h
 create mode 100644 qobject/json-parser-int.h
 create mode 100644 tests/qmp-cmd-test.c

Comments

no-reply@patchew.org Aug. 18, 2018, 10:02 a.m. UTC | #1
Hi,

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

Type: series
Message-id: 20180817150559.16243-1-armbru@redhat.com
Subject: [Qemu-devel] [PATCH v2 00/60] json: Fixes, error reporting improvements, cleanups

=== 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
git config --local diff.algorithm histogram

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'
884c1e75d7 json: Support %% in JSON strings when interpolating
5a26a0f2bd json: Improve safety of qobject_from_jsonf_nofail() & friends
639a5098e2 json: Keep interpolation state in JSONParserContext
def9c3a965 tests/drive_del-test: Fix harmless JSON interpolation bug
1c3c844f6b docs/interop/qmp-spec: How to force known good parser state
d3a2c67115 json: Clean up headers
e29f250f45 qobject: Drop superfluous includes of qemu-common.h
65addb1df9 json: Make JSONToken opaque outside json-parser.c
021baacfc3 json: Eliminate lexer state IN_WHITESPACE, pseudo-token JSON_SKIP
e7adf5381c json: Eliminate lexer state IN_ERROR and pseudo-token JSON_MIN
448709c162 json: Unbox tokens queue in JSONMessageParser
fc7874846e json: Streamline json_message_process_token()
763dce47ca json: Enforce token count and size limits more tightly
ce1419b126 qjson: Have qobject_from_json() & friends reject empty and blank
e442bf8afe json: Assert json_parser_parse() consumes all tokens on success
c62cce00e0 json: Fix streamer not to ignore trailing unterminated structures
e0b0d4e140 json: Fix latent parser aborts at end of input
41a3d27a3e qjson: Fix qobject_from_json() & friends for multiple values
d4543161e2 json: Improve names of lexer states related to numbers
1c547bc7f0 json: Nicer recovery from invalid leading zero
a16c042983 json: Replace %I64d, %I64u by %PRId64, %PRIu64
c90d2b0bb9 json: Leave rejecting invalid interpolation to parser
64773877b9 json: Pass lexical errors and limit violations to callback
4b6c4bed96 json: Treat unwanted interpolation as lexical error
9ee0cf9e7e json: Rename token JSON_ESCAPE & friends to JSON_INTERPOL
628856c8ae json: Don't create JSON_ERROR tokens that won't be used
90d6ac271a json: Don't pass null @tokens to json_parser_parse()
f4737c2067 json: Redesign the callback to consume JSON values
dfae37d338 json: Have lexer call streamer directly
5ad6f97e8f json-parser: simplify and avoid JSONParserContext allocation
de95f9aca7 json: remove useless return value from lexer/parser
a9c1d3bacc check-qjson: Fix and enable utf8_string()'s disabled part
e41b096e7c json: Fix \uXXXX for surrogate pairs
58387294e4 json: Reject invalid \uXXXX, fix \u0000
0209dbf455 json: Simplify parse_string()
0d7013b017 json: Leave rejecting invalid escape sequences to parser
0a40334aef json: Accept overlong \xC0\x80 as U+0000 ("modified UTF-8")
8d6346e0b4 json: Leave rejecting invalid UTF-8 to parser
1c2af59f08 json: Report first rather than last parse error
bbfbddf157 json: Reject invalid UTF-8 sequences
7241e8d117 check-qjson: Document we expect invalid UTF-8 to be rejected
6b94077453 json: Tighten and simplify qstring_from_escaped_str()'s loop
3ad1f50a24 json: Revamp lexer documentation
21500ab548 json: Reject unescaped control characters
627cac9476 json: Fix lexer to include the bad character in JSON_ERROR token
d0a1b08427 check-qjson: Cover interpolation more thoroughly
6fbe9e6e1e check-qjson qmp-test: Cover control characters more thoroughly
b314d87afd check-qjson: Fix utf8_string() to test all invalid sequences
310330e176 check-qjson: Simplify utf8_string()
6b869acb59 check-qjson: Cover UTF-8 in single quoted strings
2cf1f752f7 check-qjson: Consolidate partly redundant string tests
c7a4ab6392 check-qjson: Cover escaped characters more thoroughly, part 2
8f8f464015 check-qjson: Streamline escaped_string()'s test strings
5c8ed1c41f check-qjson: Cover escaped characters more thoroughly, part 1
7c223e3f0a test-qga: Clean up how we test QGA synchronization
49f1fcb4e9 qmp-test: Cover syntax and lexical errors
b9ac2b614b qmp-cmd-test: Split off qmp-test
04f1259449 check-qjson: Cover whitespace more thoroughly
74396fa7cc check-qjson: Cover blank and lexically erroneous input
1f6273a922 check-qjson: Cover multiple JSON objects in same string

=== OUTPUT BEGIN ===
Checking PATCH 1/60: check-qjson: Cover multiple JSON objects in same string...
Checking PATCH 2/60: check-qjson: Cover blank and lexically erroneous input...
Checking PATCH 3/60: check-qjson: Cover whitespace more thoroughly...
Checking PATCH 4/60: qmp-cmd-test: Split off qmp-test...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#48: 
new file mode 100644

total: 0 errors, 1 warnings, 460 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 5/60: qmp-test: Cover syntax and lexical errors...
Checking PATCH 6/60: test-qga: Clean up how we test QGA synchronization...
Checking PATCH 7/60: check-qjson: Cover escaped characters more thoroughly, part 1...
Checking PATCH 8/60: check-qjson: Streamline escaped_string()'s test strings...
Checking PATCH 9/60: check-qjson: Cover escaped characters more thoroughly, part 2...
Checking PATCH 10/60: check-qjson: Consolidate partly redundant string tests...
Checking PATCH 11/60: check-qjson: Cover UTF-8 in single quoted strings...
Checking PATCH 12/60: check-qjson: Simplify utf8_string()...
Checking PATCH 13/60: check-qjson: Fix utf8_string() to test all invalid sequences...
Checking PATCH 14/60: check-qjson qmp-test: Cover control characters more thoroughly...
Checking PATCH 15/60: check-qjson: Cover interpolation more thoroughly...
ERROR: space required after that close brace '}'
#179: FILE: tests/check-qjson.c:1122:
+            {}}));

total: 1 errors, 0 warnings, 207 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 16/60: json: Fix lexer to include the bad character in JSON_ERROR token...
Checking PATCH 17/60: json: Reject unescaped control characters...
Checking PATCH 18/60: json: Revamp lexer documentation...
Checking PATCH 19/60: json: Tighten and simplify qstring_from_escaped_str()'s loop...
Checking PATCH 20/60: check-qjson: Document we expect invalid UTF-8 to be rejected...
Checking PATCH 21/60: json: Reject invalid UTF-8 sequences...
Checking PATCH 22/60: json: Report first rather than last parse error...
Checking PATCH 23/60: json: Leave rejecting invalid UTF-8 to parser...
Checking PATCH 24/60: json: Accept overlong \xC0\x80 as U+0000 ("modified UTF-8")...
Checking PATCH 25/60: json: Leave rejecting invalid escape sequences to parser...
Checking PATCH 26/60: json: Simplify parse_string()...
Checking PATCH 27/60: json: Reject invalid \uXXXX, fix \u0000...
Checking PATCH 28/60: json: Fix \uXXXX for surrogate pairs...
Checking PATCH 29/60: check-qjson: Fix and enable utf8_string()'s disabled part...
Checking PATCH 30/60: json: remove useless return value from lexer/parser...
Checking PATCH 31/60: json-parser: simplify and avoid JSONParserContext allocation...
Checking PATCH 32/60: json: Have lexer call streamer directly...
Checking PATCH 33/60: json: Redesign the callback to consume JSON values...
Checking PATCH 34/60: json: Don't pass null @tokens to json_parser_parse()...
Checking PATCH 35/60: json: Don't create JSON_ERROR tokens that won't be used...
Checking PATCH 36/60: json: Rename token JSON_ESCAPE & friends to JSON_INTERPOL...
Checking PATCH 37/60: json: Treat unwanted interpolation as lexical error...
Checking PATCH 38/60: json: Pass lexical errors and limit violations to callback...
Checking PATCH 39/60: json: Leave rejecting invalid interpolation to parser...
Checking PATCH 40/60: json: Replace %I64d, %I64u by %PRId64, %PRIu64...
Checking PATCH 41/60: json: Nicer recovery from invalid leading zero...
Checking PATCH 42/60: json: Improve names of lexer states related to numbers...
Checking PATCH 43/60: qjson: Fix qobject_from_json() & friends for multiple values...
Checking PATCH 44/60: json: Fix latent parser aborts at end of input...
Checking PATCH 45/60: json: Fix streamer not to ignore trailing unterminated structures...
Checking PATCH 46/60: json: Assert json_parser_parse() consumes all tokens on success...
Checking PATCH 47/60: qjson: Have qobject_from_json() & friends reject empty and blank...
Checking PATCH 48/60: json: Enforce token count and size limits more tightly...
Checking PATCH 49/60: json: Streamline json_message_process_token()...
Checking PATCH 50/60: json: Unbox tokens queue in JSONMessageParser...
Checking PATCH 51/60: json: Eliminate lexer state IN_ERROR and pseudo-token JSON_MIN...
Checking PATCH 52/60: json: Eliminate lexer state IN_WHITESPACE, pseudo-token JSON_SKIP...
Checking PATCH 53/60: json: Make JSONToken opaque outside json-parser.c...
Checking PATCH 54/60: qobject: Drop superfluous includes of qemu-common.h...
Checking PATCH 55/60: json: Clean up headers...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#72: 
deleted file mode 100644

total: 0 errors, 1 warnings, 155 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 56/60: docs/interop/qmp-spec: How to force known good parser state...
Checking PATCH 57/60: tests/drive_del-test: Fix harmless JSON interpolation bug...
Checking PATCH 58/60: json: Keep interpolation state in JSONParserContext...
Checking PATCH 59/60: json: Improve safety of qobject_from_jsonf_nofail() & friends...
Checking PATCH 60/60: json: Support %% in JSON strings when interpolating...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Markus Armbruster Aug. 20, 2018, 8:31 a.m. UTC | #2
no-reply@patchew.org writes:

> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
[...]
> Checking PATCH 4/60: qmp-cmd-test: Split off qmp-test...
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #48: 
> new file mode 100644

False positive: the patch does update MAINTAINERS.  Surprising, since
commit 1a6fad0c3bd "checkpatch: reduce MAINTAINERS update message
frequency" suppresses this.  Fam, is patchew running an outdated version
of checkpatch?

>
> total: 0 errors, 1 warnings, 460 lines checked
[...]
> Checking PATCH 15/60: check-qjson: Cover interpolation more thoroughly...
> ERROR: space required after that close brace '}'
> #179: FILE: tests/check-qjson.c:1122:
> +            {}}));

I consider this a false positive.  We do require space after } in things
like } else.  We don't in complex initializes, as far as I know.

> total: 1 errors, 0 warnings, 207 lines checked
[...]
> Checking PATCH 55/60: json: Clean up headers...
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #72: 
> deleted file mode 100644

False positive: MAINTAINERS matches the deleted files' directory, and
more files remain there.

> total: 0 errors, 1 warnings, 155 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 56/60: docs/interop/qmp-spec: How to force known good parser state...
> Checking PATCH 57/60: tests/drive_del-test: Fix harmless JSON interpolation bug...
> Checking PATCH 58/60: json: Keep interpolation state in JSONParserContext...
> Checking PATCH 59/60: json: Improve safety of qobject_from_jsonf_nofail() & friends...
> Checking PATCH 60/60: json: Support %% in JSON strings when interpolating...
> === OUTPUT END ===
>
> Test command exited with code: 1
>
>
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
Fam Zheng Aug. 20, 2018, 8:42 a.m. UTC | #3
On Mon, 08/20 10:31, Markus Armbruster wrote:
> no-reply@patchew.org writes:
> 
> > Hi,
> >
> > This series seems to have some coding style problems. See output below for
> > more information:
> [...]
> > Checking PATCH 4/60: qmp-cmd-test: Split off qmp-test...
> > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> > #48: 
> > new file mode 100644
> 
> False positive: the patch does update MAINTAINERS.  Surprising, since
> commit 1a6fad0c3bd "checkpatch: reduce MAINTAINERS update message
> frequency" suppresses this.  Fam, is patchew running an outdated version
> of checkpatch?

I'm sure Patchew is using the latest checkpatch here. This is basically /me
reproducing what it does:

fam@lemon:~/work/qemu [master]$ git fetch https://github.com/patchew-project/qemu tags/patchew/20180817150559.16243-1-armbru@redhat.com
From https://github.com/patchew-project/qemu
 * tag                     patchew/20180817150559.16243-1-armbru@redhat.com -> FETCH_HEAD
fam@lemon:~/work/qemu [master]$ git checkout FETCH_HEAD
Note: checking out 'FETCH_HEAD'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at 884c1e75d7 json: Support %% in JSON strings when interpolating
fam@lemon:~/work/qemu$ git log --oneline | grep 'qmp-cmd-test: Split off qmp-test'
b9ac2b614b qmp-cmd-test: Split off qmp-test
fam@lemon:~/work/qemu$ git show --format=email b9ac2b614b | ./scripts/checkpatch.pl -
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#48:
new file mode 100644

total: 0 errors, 1 warnings, 460 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.
fam@lemon:~/work/qemu$ git log --oneline  ./scripts/checkpatch.pl | grep 1a6fad0
1a6fad0c3b checkpatch: reduce MAINTAINERS update message frequency
Markus Armbruster Aug. 20, 2018, 11:59 a.m. UTC | #4
Fam Zheng <famz@redhat.com> writes:

> On Mon, 08/20 10:31, Markus Armbruster wrote:
>> no-reply@patchew.org writes:
>> 
>> > Hi,
>> >
>> > This series seems to have some coding style problems. See output below for
>> > more information:
>> [...]
>> > Checking PATCH 4/60: qmp-cmd-test: Split off qmp-test...
>> > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
>> > #48: 
>> > new file mode 100644
>> 
>> False positive: the patch does update MAINTAINERS.  Surprising, since
>> commit 1a6fad0c3bd "checkpatch: reduce MAINTAINERS update message
>> frequency" suppresses this.  Fam, is patchew running an outdated version
>> of checkpatch?
>
> I'm sure Patchew is using the latest checkpatch here. This is basically /me
> reproducing what it does:
>
> fam@lemon:~/work/qemu [master]$ git fetch https://github.com/patchew-project/qemu tags/patchew/20180817150559.16243-1-armbru@redhat.com
>>From https://github.com/patchew-project/qemu
>  * tag                     patchew/20180817150559.16243-1-armbru@redhat.com -> FETCH_HEAD
> fam@lemon:~/work/qemu [master]$ git checkout FETCH_HEAD
> Note: checking out 'FETCH_HEAD'.
>
> You are in 'detached HEAD' state. You can look around, make experimental
> changes and commit them, and you can discard any commits you make in this
> state without impacting any branches by performing another checkout.
>
> If you want to create a new branch to retain commits you create, you may
> do so (now or later) by using -b with the checkout command again. Example:
>
>   git checkout -b <new-branch-name>
>
> HEAD is now at 884c1e75d7 json: Support %% in JSON strings when interpolating
> fam@lemon:~/work/qemu$ git log --oneline | grep 'qmp-cmd-test: Split off qmp-test'
> b9ac2b614b qmp-cmd-test: Split off qmp-test
> fam@lemon:~/work/qemu$ git show --format=email b9ac2b614b | ./scripts/checkpatch.pl -
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #48:
> new file mode 100644
>
> total: 0 errors, 1 warnings, 460 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.
> fam@lemon:~/work/qemu$ git log --oneline  ./scripts/checkpatch.pl | grep 1a6fad0
> 1a6fad0c3b checkpatch: reduce MAINTAINERS update message frequency

Okay, I can reproduce the warning the way patchew runs checkpatch:

    $ git-show --format=email 4552d915d3 | ./scripts/checkpatch.pl -
    WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
    #47: 
    new file mode 100644

    total: 0 errors, 1 warnings, 460 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.

However:

    $ git-format-patch -1 4552d915d3
    0001-qmp-cmd-test-Split-off-qmp-test.patch
    $ ./scripts/checkpatch.pl 0001-qmp-cmd-test-Split-off-qmp-test.patch 
    total: 0 errors, 0 warnings, 460 lines checked

    0001-qmp-cmd-test-Split-off-qmp-test.patch has no obvious style problems and is ready for submission.

The difference between the two: git-show --format=email lacks diffstat.

    $ git-show --format=email 4552d915d3 | diff - 0001-qmp-cmd-test-Split-off-qmp-test.patch 
    11a12,18
    > ---
    >  MAINTAINERS            |   1 +
    >  tests/Makefile.include |   3 +
    >  tests/qmp-cmd-test.c   | 213 +++++++++++++++++++++++++++++++++++++++++
    >  tests/qmp-test.c       | 193 +------------------------------------
    >  4 files changed, 219 insertions(+), 191 deletions(-)
    >  create mode 100644 tests/qmp-cmd-test.c
    497a505,507
    > -- 
    > 2.17.1
    >