Message ID | 20180817150559.16243-1-armbru@redhat.com |
---|---|
Headers | show |
Series | json: Fixes, error reporting improvements, cleanups | expand |
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
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
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
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 >