include/monitor/monitor.h | 8 +- monitor/monitor_int.h | 207 ++ hmp.c | 4 +- monitor.c | 4727 ------------------------------------- monitor/core.c | 604 +++++ monitor/hmp.c | 1351 +++++++++++ monitor/misc.c | 2406 +++++++++++++++++++ monitor/qmp.c | 404 ++++ Makefile.objs | 1 + Makefile.target | 3 +- monitor/Makefile.objs | 2 + 11 files changed, 4986 insertions(+), 4731 deletions(-) create mode 100644 monitor/monitor_int.h delete mode 100644 monitor.c create mode 100644 monitor/core.c create mode 100644 monitor/hmp.c create mode 100644 monitor/misc.c create mode 100644 monitor/qmp.c create mode 100644 monitor/Makefile.objs
monitor.c mixes a lot of different things in a single file: The core monitor infrastructure, HMP infrastrcture, QMP infrastructure, and the implementation of several HMP and QMP commands. Almost worse, struct Monitor mixes state for HMP, for QMP, and state actually shared between all monitors. monitor.c must be linked with a system emulator and even requires per-target compilation because some of the commands it implements access system emulator state. The reason why I care about this is that I'm working on a protoype for a storage daemon, which wants to use QMP (but probably not HMP) and obviously doesn't have any system emulator state. So I'm interested in some core monitor parts that can be linked to non-system-emulator tools. This series first creates separate structs MonitorQMP and MonitorHMP which inherit from Monitor, and then moves the associated infrastructure code into separate source files. While the split is probably not perfect, I think it's an improvement of the current state even for QEMU proper, and it's good enough so I can link my storage daemon against just monitor/core.o and monitor/qmp.o and get a useless QMP monitor that parses the JSON input and rejects everything as an unknown command. Next I'll try to teach it a subset of QMP commands that can actually be supported in a tool, but while there will be a few follow-up patches to achieve this, I don't expect that this work will bring up much that needs to be changed in the splitting process done in this series. Kevin Wolf (10): monitor: Remove unused password prompting fields monitor: Split monitor_init in HMP and QMP function monitor: Make MonitorQMP a child class of Monitor monitor: Create MonitorHMP with readline state monitor: Move cmd_table to MonitorHMP Move monitor.c to monitor/misc.c monitor: Create monitor_int.h with common definitions monitor: Split out monitor/qmp.c monitor: Split out monitor/hmp.c monitor: Split out monitor/core.c include/monitor/monitor.h | 8 +- monitor/monitor_int.h | 207 ++ hmp.c | 4 +- monitor.c | 4727 ------------------------------------- monitor/core.c | 604 +++++ monitor/hmp.c | 1351 +++++++++++ monitor/misc.c | 2406 +++++++++++++++++++ monitor/qmp.c | 404 ++++ Makefile.objs | 1 + Makefile.target | 3 +- monitor/Makefile.objs | 2 + 11 files changed, 4986 insertions(+), 4731 deletions(-) create mode 100644 monitor/monitor_int.h delete mode 100644 monitor.c create mode 100644 monitor/core.c create mode 100644 monitor/hmp.c create mode 100644 monitor/misc.c create mode 100644 monitor/qmp.c create mode 100644 monitor/Makefile.objs -- 2.20.1
Patchew URL: https://patchew.org/QEMU/20190607135430.22149-1-kwolf@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [RFC PATCH 00/10] monitor: Split monitor.c in core/HMP/QMP/misc
Message-id: 20190607135430.22149-1-kwolf@redhat.com
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
dbb0f22 monitor: Split out monitor/core.c
54736ce monitor: Split out monitor/hmp.c
9a88f45 monitor: Split out monitor/qmp.c
d3d699e monitor: Create monitor_int.h with common definitions
febc650 Move monitor.c to monitor/misc.c
8f3621b monitor: Move cmd_table to MonitorHMP
75ae78b monitor: Create MonitorHMP with readline state
6ab73c3 monitor: Make MonitorQMP a child class of Monitor
95f9e11 monitor: Split monitor_init in HMP and QMP function
d319053 monitor: Remove unused password prompting fields
=== OUTPUT BEGIN ===
1/10 Checking commit d3190532cbbb (monitor: Remove unused password prompting fields)
2/10 Checking commit 95f9e11f320f (monitor: Split monitor_init in HMP and QMP function)
3/10 Checking commit 6ab73c33845d (monitor: Make MonitorQMP a child class of Monitor)
4/10 Checking commit 75ae78baf77e (monitor: Create MonitorHMP with readline state)
ERROR: braces {} are necessary for all arms of this statement
#192: FILE: monitor.c:1355:
+ if (!hmp_mon->rs)
[...]
total: 1 errors, 0 warnings, 373 lines checked
Patch 4/10 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/10 Checking commit 8f3621b035e3 (monitor: Move cmd_table to MonitorHMP)
6/10 Checking commit febc6502f515 (Move monitor.c to monitor/misc.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#31:
new file mode 100644
total: 0 errors, 1 warnings, 12 lines checked
Patch 6/10 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/10 Checking commit d3d699e3b265 (monitor: Create monitor_int.h with common definitions)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#150:
new file mode 100644
WARNING: Block comments use a leading /* on a separate line
#233: FILE: monitor/monitor_int.h:79:
+ /* @sub_table is a list of 2nd level of commands. If it does not exist,
total: 0 errors, 2 warnings, 275 lines checked
Patch 7/10 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/10 Checking commit 9a88f45205d1 (monitor: Split out monitor/qmp.c)
ERROR: return is not a function, parentheses are not required
#584: FILE: monitor/monitor_int.h:153:
+ return (mon->flags & MONITOR_USE_CONTROL);
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#608:
new file mode 100644
total: 1 errors, 1 warnings, 956 lines checked
Patch 8/10 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/10 Checking commit 54736ce87811 (monitor: Split out monitor/hmp.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#39:
new file mode 100644
ERROR: braces {} are necessary for all arms of this statement
#327: FILE: monitor/hmp.c:284:
+ while (qemu_isspace(*pch))
[...]
ERROR: space required before the open parenthesis '('
#340: FILE: monitor/hmp.c:297:
+ switch(*pch) {
ERROR: braces {} are necessary for all arms of this statement
#363: FILE: monitor/hmp.c:320:
+ if (*pch == '\0')
[...]
ERROR: braces {} are necessary for all arms of this statement
#367: FILE: monitor/hmp.c:324:
+ if (*pch != '\'')
[...]
ERROR: braces {} are necessary for all arms of this statement
#382: FILE: monitor/hmp.c:339:
+ if ((q - buf) < sizeof(buf) - 1)
[...]
ERROR: braces {} are necessary for all arms of this statement
#386: FILE: monitor/hmp.c:343:
+ while (qemu_isspace(*pch))
[...]
ERROR: braces {} are necessary for all arms of this statement
#390: FILE: monitor/hmp.c:347:
+ if (ret < 0)
[...]
ERROR: consider using qemu_strtoull in preference to strtoull
#401: FILE: monitor/hmp.c:358:
+ n = strtoull(pch, &p, 0);
ERROR: braces {} are necessary for all arms of this statement
#409: FILE: monitor/hmp.c:366:
+ while (qemu_isspace(*pch))
[...]
ERROR: space required before the open parenthesis '('
#422: FILE: monitor/hmp.c:379:
+ for(;;) {
ERROR: braces {} are necessary for all arms of this statement
#424: FILE: monitor/hmp.c:381:
+ if (op != '*' && op != '/' && op != '%')
[...]
ERROR: space required before the open parenthesis '('
#428: FILE: monitor/hmp.c:385:
+ switch(op) {
ERROR: braces {} are necessary for all arms of this statement
#435: FILE: monitor/hmp.c:392:
+ if (val2 == 0)
[...]
ERROR: braces {} are necessary for all arms of this statement
#437: FILE: monitor/hmp.c:394:
+ if (op == '/')
[...]
+ else
[...]
ERROR: space required before the open parenthesis '('
#453: FILE: monitor/hmp.c:410:
+ for(;;) {
ERROR: braces {} are necessary for all arms of this statement
#455: FILE: monitor/hmp.c:412:
+ if (op != '&' && op != '|' && op != '^')
[...]
ERROR: space required before the open parenthesis '('
#459: FILE: monitor/hmp.c:416:
+ switch(op) {
ERROR: space required before the open parenthesis '('
#481: FILE: monitor/hmp.c:438:
+ for(;;) {
ERROR: braces {} are necessary for all arms of this statement
#483: FILE: monitor/hmp.c:440:
+ if (op != '+' && op != '-')
[...]
ERROR: braces {} are necessary for all arms of this statement
#487: FILE: monitor/hmp.c:444:
+ if (op == '+')
[...]
+ else
[...]
ERROR: braces {} are necessary for all arms of this statement
#502: FILE: monitor/hmp.c:459:
+ while (qemu_isspace(*pch))
[...]
ERROR: braces {} are necessary for all arms of this statement
#541: FILE: monitor/hmp.c:498:
+ while (qemu_isspace(*p))
[...]
ERROR: braces {} are necessary for all arms of this statement
#543: FILE: monitor/hmp.c:500:
+ if (*p == '\0')
[...]
ERROR: braces {} are necessary for all arms of this statement
#546: FILE: monitor/hmp.c:503:
+ while (*p != '\0' && *p != '/' && !qemu_isspace(*p))
[...]
ERROR: braces {} are necessary for all arms of this statement
#549: FILE: monitor/hmp.c:506:
+ if (len > nlen - 1)
[...]
ERROR: braces {} are necessary for all arms of this statement
#565: FILE: monitor/hmp.c:522:
+ if (*type == ',')
[...]
ERROR: space required before the open parenthesis '('
#684: FILE: monitor/hmp.c:641:
+ for(;;) {
ERROR: space required before the open parenthesis '('
#691: FILE: monitor/hmp.c:648:
+ switch(c) {
ERROR: braces {} are necessary for all arms of this statement
#698: FILE: monitor/hmp.c:655:
+ while (qemu_isspace(*p))
[...]
ERROR: space required before the open parenthesis '('
#709: FILE: monitor/hmp.c:666:
+ switch(c) {
ERROR: braces {} are necessary for all arms of this statement
#757: FILE: monitor/hmp.c:714:
+ while (qemu_isspace(*p))
[...]
ERROR: space required before the open parenthesis '('
#772: FILE: monitor/hmp.c:729:
+ for(;;) {
ERROR: space required before the open parenthesis '('
#773: FILE: monitor/hmp.c:730:
+ switch(*p) {
ERROR: braces {} are necessary for all arms of this statement
#840: FILE: monitor/hmp.c:797:
+ while (qemu_isspace(*p))
[...]
ERROR: braces {} are necessary for all arms of this statement
#861: FILE: monitor/hmp.c:818:
+ if (get_expr(mon, &val, &p))
[...]
ERROR: braces {} are necessary for all arms of this statement
#965: FILE: monitor/hmp.c:922:
+ if (c == '\0')
[...]
ERROR: braces {} are necessary for all arms of this statement
#967: FILE: monitor/hmp.c:924:
+ while (qemu_isspace(*p))
[...]
ERROR: space required before the open parenthesis '('
#971: FILE: monitor/hmp.c:928:
+ if(c != *p) {
ERROR: space required before the open parenthesis '('
#972: FILE: monitor/hmp.c:929:
+ if(!is_valid_option(p, typestr)) {
ERROR: space required before the open parenthesis '('
#980: FILE: monitor/hmp.c:937:
+ if(skip_key) {
ERROR: braces {} are necessary for all arms of this statement
#1024: FILE: monitor/hmp.c:981:
+ while (qemu_isspace(*p))
[...]
ERROR: space required before the open parenthesis '('
#1074: FILE: monitor/hmp.c:1031:
+ for(;;) {
ERROR: braces {} are necessary for all arms of this statement
#1078: FILE: monitor/hmp.c:1035:
+ if (len > sizeof(cmd) - 2)
[...]
ERROR: braces {} are necessary for all arms of this statement
#1085: FILE: monitor/hmp.c:1042:
+ if (*p == '\0')
[...]
ERROR: braces {} are necessary for all arms of this statement
#1108: FILE: monitor/hmp.c:1065:
+ if (input_path_len > sizeof(path) - 1)
[...]
ERROR: braces {} are necessary for all arms of this statement
#1115: FILE: monitor/hmp.c:1072:
+ if (!ffs)
[...]
ERROR: space required before the open parenthesis '('
#1117: FILE: monitor/hmp.c:1074:
+ for(;;) {
ERROR: braces {} are necessary for all arms of this statement
#1120: FILE: monitor/hmp.c:1077:
+ if (!d)
[...]
WARNING: Block comments use a leading /* on a separate line
#1132: FILE: monitor/hmp.c:1089:
+ /* stat the file to find out if it's a directory.
ERROR: braces {} are necessary for all arms of this statement
#1163: FILE: monitor/hmp.c:1120:
+ if (nb_args == 0)
[...]
+ else
[...]
ERROR: space required before the open parenthesis '('
#1199: FILE: monitor/hmp.c:1156:
+ for(i = 0; i < nb_args - 2; i++) {
ERROR: braces {} are necessary for all arms of this statement
#1202: FILE: monitor/hmp.c:1159:
+ while (*ptype == '?')
[...]
ERROR: space required before the open parenthesis '('
#1212: FILE: monitor/hmp.c:1169:
+ switch(*ptype) {
WARNING: Block comments use a leading /* on a separate line
#1254: FILE: monitor/hmp.c:1211:
+ /* if the line ends with a space, it means we want to complete the
WARNING: Block comments use * on subsequent lines
#1255: FILE: monitor/hmp.c:1212:
+ /* if the line ends with a space, it means we want to complete the
+ next arg */
WARNING: Block comments use a trailing */ on a separate line
#1255: FILE: monitor/hmp.c:1212:
+ next arg */
WARNING: Block comments use a leading /* on a separate line
#1348: FILE: monitor/hmp.c:1305:
+/* These functions just adapt the readline interface in a typesafe way. We
ERROR: space required before the open parenthesis '('
#2850: FILE: monitor/monitor_int.h:166:
+ for(;;) {
total: 53 errors, 6 warnings, 2828 lines checked
Patch 9/10 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/10 Checking commit dbb0f228d63f (monitor: Split out monitor/core.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#44:
new file mode 100644
ERROR: braces {} are necessary for all arms of this statement
#214: FILE: monitor/core.c:166:
+ if (!mon)
[...]
WARNING: Block comments use a leading /* on a separate line
#650: FILE: monitor/core.c:602:
+ { /* end of list */ }
total: 1 errors, 2 warnings, 1242 lines checked
Patch 10/10 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
The full log is available at
http://patchew.org/logs/20190607135430.22149-1-kwolf@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Patchew URL: https://patchew.org/QEMU/20190607135430.22149-1-kwolf@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [RFC PATCH 00/10] monitor: Split monitor.c in core/HMP/QMP/misc
Message-id: 20190607135430.22149-1-kwolf@redhat.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20190607184802.100945-1-vsementsov@virtuozzo.com -> patchew/20190607184802.100945-1-vsementsov@virtuozzo.com
Switched to a new branch 'test'
393e03b monitor: Split out monitor/core.c
6938636 monitor: Split out monitor/hmp.c
b33f363 monitor: Split out monitor/qmp.c
4e784bf monitor: Create monitor_int.h with common definitions
ec11ead Move monitor.c to monitor/misc.c
00365cc monitor: Move cmd_table to MonitorHMP
05c3aa3 monitor: Create MonitorHMP with readline state
5435f06 monitor: Make MonitorQMP a child class of Monitor
a541d49 monitor: Split monitor_init in HMP and QMP function
d1dcf9d monitor: Remove unused password prompting fields
=== OUTPUT BEGIN ===
1/10 Checking commit d1dcf9d89219 (monitor: Remove unused password prompting fields)
2/10 Checking commit a541d49ece87 (monitor: Split monitor_init in HMP and QMP function)
3/10 Checking commit 5435f06ca5bf (monitor: Make MonitorQMP a child class of Monitor)
4/10 Checking commit 05c3aa3627dd (monitor: Create MonitorHMP with readline state)
ERROR: braces {} are necessary for all arms of this statement
#192: FILE: monitor.c:1355:
+ if (!hmp_mon->rs)
[...]
total: 1 errors, 0 warnings, 373 lines checked
Patch 4/10 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/10 Checking commit 00365cc53224 (monitor: Move cmd_table to MonitorHMP)
6/10 Checking commit ec11ead0d0aa (Move monitor.c to monitor/misc.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#31:
new file mode 100644
total: 0 errors, 1 warnings, 12 lines checked
Patch 6/10 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/10 Checking commit 4e784bfbeaf0 (monitor: Create monitor_int.h with common definitions)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#150:
new file mode 100644
WARNING: Block comments use a leading /* on a separate line
#233: FILE: monitor/monitor_int.h:79:
+ /* @sub_table is a list of 2nd level of commands. If it does not exist,
total: 0 errors, 2 warnings, 275 lines checked
Patch 7/10 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/10 Checking commit b33f3634c4a3 (monitor: Split out monitor/qmp.c)
ERROR: return is not a function, parentheses are not required
#584: FILE: monitor/monitor_int.h:153:
+ return (mon->flags & MONITOR_USE_CONTROL);
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#608:
new file mode 100644
total: 1 errors, 1 warnings, 956 lines checked
Patch 8/10 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/10 Checking commit 693863658d05 (monitor: Split out monitor/hmp.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#39:
new file mode 100644
ERROR: braces {} are necessary for all arms of this statement
#327: FILE: monitor/hmp.c:284:
+ while (qemu_isspace(*pch))
[...]
ERROR: space required before the open parenthesis '('
#340: FILE: monitor/hmp.c:297:
+ switch(*pch) {
ERROR: braces {} are necessary for all arms of this statement
#363: FILE: monitor/hmp.c:320:
+ if (*pch == '\0')
[...]
ERROR: braces {} are necessary for all arms of this statement
#367: FILE: monitor/hmp.c:324:
+ if (*pch != '\'')
[...]
ERROR: braces {} are necessary for all arms of this statement
#382: FILE: monitor/hmp.c:339:
+ if ((q - buf) < sizeof(buf) - 1)
[...]
ERROR: braces {} are necessary for all arms of this statement
#386: FILE: monitor/hmp.c:343:
+ while (qemu_isspace(*pch))
[...]
ERROR: braces {} are necessary for all arms of this statement
#390: FILE: monitor/hmp.c:347:
+ if (ret < 0)
[...]
ERROR: consider using qemu_strtoull in preference to strtoull
#401: FILE: monitor/hmp.c:358:
+ n = strtoull(pch, &p, 0);
ERROR: braces {} are necessary for all arms of this statement
#409: FILE: monitor/hmp.c:366:
+ while (qemu_isspace(*pch))
[...]
ERROR: space required before the open parenthesis '('
#422: FILE: monitor/hmp.c:379:
+ for(;;) {
ERROR: braces {} are necessary for all arms of this statement
#424: FILE: monitor/hmp.c:381:
+ if (op != '*' && op != '/' && op != '%')
[...]
ERROR: space required before the open parenthesis '('
#428: FILE: monitor/hmp.c:385:
+ switch(op) {
ERROR: braces {} are necessary for all arms of this statement
#435: FILE: monitor/hmp.c:392:
+ if (val2 == 0)
[...]
ERROR: braces {} are necessary for all arms of this statement
#437: FILE: monitor/hmp.c:394:
+ if (op == '/')
[...]
+ else
[...]
ERROR: space required before the open parenthesis '('
#453: FILE: monitor/hmp.c:410:
+ for(;;) {
ERROR: braces {} are necessary for all arms of this statement
#455: FILE: monitor/hmp.c:412:
+ if (op != '&' && op != '|' && op != '^')
[...]
ERROR: space required before the open parenthesis '('
#459: FILE: monitor/hmp.c:416:
+ switch(op) {
ERROR: space required before the open parenthesis '('
#481: FILE: monitor/hmp.c:438:
+ for(;;) {
ERROR: braces {} are necessary for all arms of this statement
#483: FILE: monitor/hmp.c:440:
+ if (op != '+' && op != '-')
[...]
ERROR: braces {} are necessary for all arms of this statement
#487: FILE: monitor/hmp.c:444:
+ if (op == '+')
[...]
+ else
[...]
ERROR: braces {} are necessary for all arms of this statement
#502: FILE: monitor/hmp.c:459:
+ while (qemu_isspace(*pch))
[...]
ERROR: braces {} are necessary for all arms of this statement
#541: FILE: monitor/hmp.c:498:
+ while (qemu_isspace(*p))
[...]
ERROR: braces {} are necessary for all arms of this statement
#543: FILE: monitor/hmp.c:500:
+ if (*p == '\0')
[...]
ERROR: braces {} are necessary for all arms of this statement
#546: FILE: monitor/hmp.c:503:
+ while (*p != '\0' && *p != '/' && !qemu_isspace(*p))
[...]
ERROR: braces {} are necessary for all arms of this statement
#549: FILE: monitor/hmp.c:506:
+ if (len > nlen - 1)
[...]
ERROR: braces {} are necessary for all arms of this statement
#565: FILE: monitor/hmp.c:522:
+ if (*type == ',')
[...]
ERROR: space required before the open parenthesis '('
#684: FILE: monitor/hmp.c:641:
+ for(;;) {
ERROR: space required before the open parenthesis '('
#691: FILE: monitor/hmp.c:648:
+ switch(c) {
ERROR: braces {} are necessary for all arms of this statement
#698: FILE: monitor/hmp.c:655:
+ while (qemu_isspace(*p))
[...]
ERROR: space required before the open parenthesis '('
#709: FILE: monitor/hmp.c:666:
+ switch(c) {
ERROR: braces {} are necessary for all arms of this statement
#757: FILE: monitor/hmp.c:714:
+ while (qemu_isspace(*p))
[...]
ERROR: space required before the open parenthesis '('
#772: FILE: monitor/hmp.c:729:
+ for(;;) {
ERROR: space required before the open parenthesis '('
#773: FILE: monitor/hmp.c:730:
+ switch(*p) {
ERROR: braces {} are necessary for all arms of this statement
#840: FILE: monitor/hmp.c:797:
+ while (qemu_isspace(*p))
[...]
ERROR: braces {} are necessary for all arms of this statement
#861: FILE: monitor/hmp.c:818:
+ if (get_expr(mon, &val, &p))
[...]
ERROR: braces {} are necessary for all arms of this statement
#965: FILE: monitor/hmp.c:922:
+ if (c == '\0')
[...]
ERROR: braces {} are necessary for all arms of this statement
#967: FILE: monitor/hmp.c:924:
+ while (qemu_isspace(*p))
[...]
ERROR: space required before the open parenthesis '('
#971: FILE: monitor/hmp.c:928:
+ if(c != *p) {
ERROR: space required before the open parenthesis '('
#972: FILE: monitor/hmp.c:929:
+ if(!is_valid_option(p, typestr)) {
ERROR: space required before the open parenthesis '('
#980: FILE: monitor/hmp.c:937:
+ if(skip_key) {
ERROR: braces {} are necessary for all arms of this statement
#1024: FILE: monitor/hmp.c:981:
+ while (qemu_isspace(*p))
[...]
ERROR: space required before the open parenthesis '('
#1074: FILE: monitor/hmp.c:1031:
+ for(;;) {
ERROR: braces {} are necessary for all arms of this statement
#1078: FILE: monitor/hmp.c:1035:
+ if (len > sizeof(cmd) - 2)
[...]
ERROR: braces {} are necessary for all arms of this statement
#1085: FILE: monitor/hmp.c:1042:
+ if (*p == '\0')
[...]
ERROR: braces {} are necessary for all arms of this statement
#1108: FILE: monitor/hmp.c:1065:
+ if (input_path_len > sizeof(path) - 1)
[...]
ERROR: braces {} are necessary for all arms of this statement
#1115: FILE: monitor/hmp.c:1072:
+ if (!ffs)
[...]
ERROR: space required before the open parenthesis '('
#1117: FILE: monitor/hmp.c:1074:
+ for(;;) {
ERROR: braces {} are necessary for all arms of this statement
#1120: FILE: monitor/hmp.c:1077:
+ if (!d)
[...]
WARNING: Block comments use a leading /* on a separate line
#1132: FILE: monitor/hmp.c:1089:
+ /* stat the file to find out if it's a directory.
ERROR: braces {} are necessary for all arms of this statement
#1163: FILE: monitor/hmp.c:1120:
+ if (nb_args == 0)
[...]
+ else
[...]
ERROR: space required before the open parenthesis '('
#1199: FILE: monitor/hmp.c:1156:
+ for(i = 0; i < nb_args - 2; i++) {
ERROR: braces {} are necessary for all arms of this statement
#1202: FILE: monitor/hmp.c:1159:
+ while (*ptype == '?')
[...]
ERROR: space required before the open parenthesis '('
#1212: FILE: monitor/hmp.c:1169:
+ switch(*ptype) {
WARNING: Block comments use a leading /* on a separate line
#1254: FILE: monitor/hmp.c:1211:
+ /* if the line ends with a space, it means we want to complete the
WARNING: Block comments use * on subsequent lines
#1255: FILE: monitor/hmp.c:1212:
+ /* if the line ends with a space, it means we want to complete the
+ next arg */
WARNING: Block comments use a trailing */ on a separate line
#1255: FILE: monitor/hmp.c:1212:
+ next arg */
WARNING: Block comments use a leading /* on a separate line
#1348: FILE: monitor/hmp.c:1305:
+/* These functions just adapt the readline interface in a typesafe way. We
ERROR: space required before the open parenthesis '('
#2850: FILE: monitor/monitor_int.h:166:
+ for(;;) {
total: 53 errors, 6 warnings, 2828 lines checked
Patch 9/10 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/10 Checking commit 393e03bb4f4e (monitor: Split out monitor/core.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#44:
new file mode 100644
ERROR: braces {} are necessary for all arms of this statement
#214: FILE: monitor/core.c:166:
+ if (!mon)
[...]
WARNING: Block comments use a leading /* on a separate line
#650: FILE: monitor/core.c:602:
+ { /* end of list */ }
total: 1 errors, 2 warnings, 1242 lines checked
Patch 10/10 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
The full log is available at
http://patchew.org/logs/20190607135430.22149-1-kwolf@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On Fri, Jun 07, 2019 at 03:54:20PM +0200, Kevin Wolf wrote: > monitor.c mixes a lot of different things in a single file: The core > monitor infrastructure, HMP infrastrcture, QMP infrastructure, and the > implementation of several HMP and QMP commands. Almost worse, struct > Monitor mixes state for HMP, for QMP, and state actually shared between > all monitors. monitor.c must be linked with a system emulator and even > requires per-target compilation because some of the commands it > implements access system emulator state. > > The reason why I care about this is that I'm working on a protoype for a > storage daemon, which wants to use QMP (but probably not HMP) and > obviously doesn't have any system emulator state. So I'm interested in > some core monitor parts that can be linked to non-system-emulator tools. > > This series first creates separate structs MonitorQMP and MonitorHMP > which inherit from Monitor, and then moves the associated infrastructure > code into separate source files. > > While the split is probably not perfect, I think it's an improvement of > the current state even for QEMU proper, and it's good enough so I can > link my storage daemon against just monitor/core.o and monitor/qmp.o and > get a useless QMP monitor that parses the JSON input and rejects > everything as an unknown command. > > Next I'll try to teach it a subset of QMP commands that can actually be > supported in a tool, but while there will be a few follow-up patches to > achieve this, I don't expect that this work will bring up much that > needs to be changed in the splitting process done in this series. > > Kevin Wolf (10): > monitor: Remove unused password prompting fields > monitor: Split monitor_init in HMP and QMP function > monitor: Make MonitorQMP a child class of Monitor > monitor: Create MonitorHMP with readline state > monitor: Move cmd_table to MonitorHMP > Move monitor.c to monitor/misc.c > monitor: Create monitor_int.h with common definitions > monitor: Split out monitor/qmp.c > monitor: Split out monitor/hmp.c > monitor: Split out monitor/core.c > > include/monitor/monitor.h | 8 +- > monitor/monitor_int.h | 207 ++ > hmp.c | 4 +- > monitor.c | 4727 ------------------------------------- > monitor/core.c | 604 +++++ > monitor/hmp.c | 1351 +++++++++++ > monitor/misc.c | 2406 +++++++++++++++++++ > monitor/qmp.c | 404 ++++ > Makefile.objs | 1 + > Makefile.target | 3 +- > monitor/Makefile.objs | 2 + It will be nice to have the monitor code split up a bit more. I'm not a fan, however, of having both $ROOT/qmp.c and $ROOT/monitor/qmp.c Likwise $ROOT/hmp.c and $ROOT/monitor/hmp.c. Can we move those other existing files out of the root dir, into monitor/, so we don't have two files with the same name in different dirs. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Am 07.06.2019 um 16:03 hat Daniel P. Berrangé geschrieben:
> On Fri, Jun 07, 2019 at 03:54:20PM +0200, Kevin Wolf wrote:
> > monitor.c mixes a lot of different things in a single file: The core
> > monitor infrastructure, HMP infrastrcture, QMP infrastructure, and the
> > implementation of several HMP and QMP commands. Almost worse, struct
> > Monitor mixes state for HMP, for QMP, and state actually shared between
> > all monitors. monitor.c must be linked with a system emulator and even
> > requires per-target compilation because some of the commands it
> > implements access system emulator state.
> >
> > The reason why I care about this is that I'm working on a protoype for a
> > storage daemon, which wants to use QMP (but probably not HMP) and
> > obviously doesn't have any system emulator state. So I'm interested in
> > some core monitor parts that can be linked to non-system-emulator tools.
> >
> > This series first creates separate structs MonitorQMP and MonitorHMP
> > which inherit from Monitor, and then moves the associated infrastructure
> > code into separate source files.
> >
> > While the split is probably not perfect, I think it's an improvement of
> > the current state even for QEMU proper, and it's good enough so I can
> > link my storage daemon against just monitor/core.o and monitor/qmp.o and
> > get a useless QMP monitor that parses the JSON input and rejects
> > everything as an unknown command.
> >
> > Next I'll try to teach it a subset of QMP commands that can actually be
> > supported in a tool, but while there will be a few follow-up patches to
> > achieve this, I don't expect that this work will bring up much that
> > needs to be changed in the splitting process done in this series.
> >
> > Kevin Wolf (10):
> > monitor: Remove unused password prompting fields
> > monitor: Split monitor_init in HMP and QMP function
> > monitor: Make MonitorQMP a child class of Monitor
> > monitor: Create MonitorHMP with readline state
> > monitor: Move cmd_table to MonitorHMP
> > Move monitor.c to monitor/misc.c
> > monitor: Create monitor_int.h with common definitions
> > monitor: Split out monitor/qmp.c
> > monitor: Split out monitor/hmp.c
> > monitor: Split out monitor/core.c
> >
> > include/monitor/monitor.h | 8 +-
> > monitor/monitor_int.h | 207 ++
> > hmp.c | 4 +-
> > monitor.c | 4727 -------------------------------------
> > monitor/core.c | 604 +++++
> > monitor/hmp.c | 1351 +++++++++++
> > monitor/misc.c | 2406 +++++++++++++++++++
> > monitor/qmp.c | 404 ++++
> > Makefile.objs | 1 +
> > Makefile.target | 3 +-
> > monitor/Makefile.objs | 2 +
>
> It will be nice to have the monitor code split up a bit more.
>
> I'm not a fan, however, of having both $ROOT/qmp.c and $ROOT/monitor/qmp.c
> Likwise $ROOT/hmp.c and $ROOT/monitor/hmp.c. Can we move those other
> existing files out of the root dir, into monitor/, so we don't have two
> files with the same name in different dirs.
$ROOT/hmp.c and $ROOT/qmp.c contain various command implementations,
just as $ROOT/monitor/misc.c. This is still a bit of a mess. I'll have
to address this at least partially in the next step because I need to
separate commands that can be linked with tools from those that require
a system emulator.
My plan involves at least creating some monitor/qmp-cmds-*.c, which
might already make $ROOT/qmp.c empty. Even though I don't strictly need
it, there's no reason not to do the same for HMP, too. In any case, I'd
rather address this in a separate follow-up series.
But if people prefer, I can move the existing files in the root
directory to monitor/{qmp,hmp}-cmds.c temporarily in this series and
then work from there with follow-ups until they are empty (or maybe I
don't even have to make them completely empty then).
Kevin
On Fri, Jun 07, 2019 at 04:25:14PM +0200, Kevin Wolf wrote:
> Am 07.06.2019 um 16:03 hat Daniel P. Berrangé geschrieben:
> > On Fri, Jun 07, 2019 at 03:54:20PM +0200, Kevin Wolf wrote:
> > > monitor.c mixes a lot of different things in a single file: The core
> > > monitor infrastructure, HMP infrastrcture, QMP infrastructure, and the
> > > implementation of several HMP and QMP commands. Almost worse, struct
> > > Monitor mixes state for HMP, for QMP, and state actually shared between
> > > all monitors. monitor.c must be linked with a system emulator and even
> > > requires per-target compilation because some of the commands it
> > > implements access system emulator state.
> > >
> > > The reason why I care about this is that I'm working on a protoype for a
> > > storage daemon, which wants to use QMP (but probably not HMP) and
> > > obviously doesn't have any system emulator state. So I'm interested in
> > > some core monitor parts that can be linked to non-system-emulator tools.
> > >
> > > This series first creates separate structs MonitorQMP and MonitorHMP
> > > which inherit from Monitor, and then moves the associated infrastructure
> > > code into separate source files.
> > >
> > > While the split is probably not perfect, I think it's an improvement of
> > > the current state even for QEMU proper, and it's good enough so I can
> > > link my storage daemon against just monitor/core.o and monitor/qmp.o and
> > > get a useless QMP monitor that parses the JSON input and rejects
> > > everything as an unknown command.
> > >
> > > Next I'll try to teach it a subset of QMP commands that can actually be
> > > supported in a tool, but while there will be a few follow-up patches to
> > > achieve this, I don't expect that this work will bring up much that
> > > needs to be changed in the splitting process done in this series.
> > >
> > > Kevin Wolf (10):
> > > monitor: Remove unused password prompting fields
> > > monitor: Split monitor_init in HMP and QMP function
> > > monitor: Make MonitorQMP a child class of Monitor
> > > monitor: Create MonitorHMP with readline state
> > > monitor: Move cmd_table to MonitorHMP
> > > Move monitor.c to monitor/misc.c
> > > monitor: Create monitor_int.h with common definitions
> > > monitor: Split out monitor/qmp.c
> > > monitor: Split out monitor/hmp.c
> > > monitor: Split out monitor/core.c
> > >
> > > include/monitor/monitor.h | 8 +-
> > > monitor/monitor_int.h | 207 ++
> > > hmp.c | 4 +-
> > > monitor.c | 4727 -------------------------------------
> > > monitor/core.c | 604 +++++
> > > monitor/hmp.c | 1351 +++++++++++
> > > monitor/misc.c | 2406 +++++++++++++++++++
> > > monitor/qmp.c | 404 ++++
> > > Makefile.objs | 1 +
> > > Makefile.target | 3 +-
> > > monitor/Makefile.objs | 2 +
> >
> > It will be nice to have the monitor code split up a bit more.
> >
> > I'm not a fan, however, of having both $ROOT/qmp.c and $ROOT/monitor/qmp.c
> > Likwise $ROOT/hmp.c and $ROOT/monitor/hmp.c. Can we move those other
> > existing files out of the root dir, into monitor/, so we don't have two
> > files with the same name in different dirs.
>
> $ROOT/hmp.c and $ROOT/qmp.c contain various command implementations,
> just as $ROOT/monitor/misc.c. This is still a bit of a mess. I'll have
> to address this at least partially in the next step because I need to
> separate commands that can be linked with tools from those that require
> a system emulator.
>
> My plan involves at least creating some monitor/qmp-cmds-*.c, which
> might already make $ROOT/qmp.c empty. Even though I don't strictly need
> it, there's no reason not to do the same for HMP, too. In any case, I'd
> rather address this in a separate follow-up series.
Ok, if you have a plan for this, that's fine with me.
> But if people prefer, I can move the existing files in the root
> directory to monitor/{qmp,hmp}-cmds.c temporarily in this series and
> then work from there with follow-ups until they are empty (or maybe I
> don't even have to make them completely empty then).
A plain rename like this won't hurt in the meantime.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Fri, Jun 07, 2019 at 04:25:14PM +0200, Kevin Wolf wrote:
> > Am 07.06.2019 um 16:03 hat Daniel P. Berrangé geschrieben:
> > > On Fri, Jun 07, 2019 at 03:54:20PM +0200, Kevin Wolf wrote:
> > > > monitor.c mixes a lot of different things in a single file: The core
> > > > monitor infrastructure, HMP infrastrcture, QMP infrastructure, and the
> > > > implementation of several HMP and QMP commands. Almost worse, struct
> > > > Monitor mixes state for HMP, for QMP, and state actually shared between
> > > > all monitors. monitor.c must be linked with a system emulator and even
> > > > requires per-target compilation because some of the commands it
> > > > implements access system emulator state.
> > > >
> > > > The reason why I care about this is that I'm working on a protoype for a
> > > > storage daemon, which wants to use QMP (but probably not HMP) and
> > > > obviously doesn't have any system emulator state. So I'm interested in
> > > > some core monitor parts that can be linked to non-system-emulator tools.
> > > >
> > > > This series first creates separate structs MonitorQMP and MonitorHMP
> > > > which inherit from Monitor, and then moves the associated infrastructure
> > > > code into separate source files.
> > > >
> > > > While the split is probably not perfect, I think it's an improvement of
> > > > the current state even for QEMU proper, and it's good enough so I can
> > > > link my storage daemon against just monitor/core.o and monitor/qmp.o and
> > > > get a useless QMP monitor that parses the JSON input and rejects
> > > > everything as an unknown command.
> > > >
> > > > Next I'll try to teach it a subset of QMP commands that can actually be
> > > > supported in a tool, but while there will be a few follow-up patches to
> > > > achieve this, I don't expect that this work will bring up much that
> > > > needs to be changed in the splitting process done in this series.
> > > >
> > > > Kevin Wolf (10):
> > > > monitor: Remove unused password prompting fields
> > > > monitor: Split monitor_init in HMP and QMP function
> > > > monitor: Make MonitorQMP a child class of Monitor
> > > > monitor: Create MonitorHMP with readline state
> > > > monitor: Move cmd_table to MonitorHMP
> > > > Move monitor.c to monitor/misc.c
> > > > monitor: Create monitor_int.h with common definitions
> > > > monitor: Split out monitor/qmp.c
> > > > monitor: Split out monitor/hmp.c
> > > > monitor: Split out monitor/core.c
> > > >
> > > > include/monitor/monitor.h | 8 +-
> > > > monitor/monitor_int.h | 207 ++
> > > > hmp.c | 4 +-
> > > > monitor.c | 4727 -------------------------------------
> > > > monitor/core.c | 604 +++++
> > > > monitor/hmp.c | 1351 +++++++++++
> > > > monitor/misc.c | 2406 +++++++++++++++++++
> > > > monitor/qmp.c | 404 ++++
> > > > Makefile.objs | 1 +
> > > > Makefile.target | 3 +-
> > > > monitor/Makefile.objs | 2 +
> > >
> > > It will be nice to have the monitor code split up a bit more.
> > >
> > > I'm not a fan, however, of having both $ROOT/qmp.c and $ROOT/monitor/qmp.c
> > > Likwise $ROOT/hmp.c and $ROOT/monitor/hmp.c. Can we move those other
> > > existing files out of the root dir, into monitor/, so we don't have two
> > > files with the same name in different dirs.
> >
> > $ROOT/hmp.c and $ROOT/qmp.c contain various command implementations,
> > just as $ROOT/monitor/misc.c. This is still a bit of a mess. I'll have
> > to address this at least partially in the next step because I need to
> > separate commands that can be linked with tools from those that require
> > a system emulator.
> >
> > My plan involves at least creating some monitor/qmp-cmds-*.c, which
> > might already make $ROOT/qmp.c empty. Even though I don't strictly need
> > it, there's no reason not to do the same for HMP, too. In any case, I'd
> > rather address this in a separate follow-up series.
>
> Ok, if you have a plan for this, that's fine with me.
>
> > But if people prefer, I can move the existing files in the root
> > directory to monitor/{qmp,hmp}-cmds.c temporarily in this series and
> > then work from there with follow-ups until they are empty (or maybe I
> > don't even have to make them completely empty then).
>
> A plain rename like this won't hurt in the meantime.
Yeh agreed, my brain hurts too much with files of the same name.
Dave
> Regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 6/7/19 10:35 AM, Dr. David Alan Gilbert wrote:
>>> But if people prefer, I can move the existing files in the root
>>> directory to monitor/{qmp,hmp}-cmds.c temporarily in this series and
>>> then work from there with follow-ups until they are empty (or maybe I
>>> don't even have to make them completely empty then).
>>
>> A plain rename like this won't hurt in the meantime.
>
> Yeh agreed, my brain hurts too much with files of the same name.
Not just that, but in gdb, it's harder to set breakpoints of the form
file.c:line if file.c is not unique to the image.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
© 2016 - 2025 Red Hat, Inc.