The only user of the qemuMonitorTestNewSimple macro is using JSON. Always pass 'true' to qemuMonitorTestNew and remove the 'json' argument. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- tests/qemumonitortestutils.c | 2 +- tests/qemumonitortestutils.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 8d7c503c6e..03261d649e 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -1318,7 +1318,7 @@ qemuMonitorTestNewFromFile(const char *fileName, if (virTestLoadFile(fileName, &json) < 0) goto cleanup; - if (simple && !(test = qemuMonitorTestNewSimple(true, xmlopt))) + if (simple && !(test = qemuMonitorTestNewSimple(xmlopt))) goto cleanup; /* Our JSON parser expects replies to be separated by a newline character. diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h index a2d2d30820..23192c1223 100644 --- a/tests/qemumonitortestutils.h +++ b/tests/qemumonitortestutils.h @@ -74,8 +74,8 @@ int qemuMonitorTestAddItemExpect(qemuMonitorTestPtr test, bool apostrophe, const char *response); -# define qemuMonitorTestNewSimple(json, xmlopt) \ - qemuMonitorTestNew(json, xmlopt, NULL, NULL, NULL, NULL) +# define qemuMonitorTestNewSimple(xmlopt) \ + qemuMonitorTestNew(true, xmlopt, NULL, NULL, NULL, NULL) # define qemuMonitorTestNewSchema(xmlopt, schema) \ qemuMonitorTestNew(true, xmlopt, NULL, NULL, NULL, schema) -- 2.20.1

Use VIR_AUTOFREE. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- tests/qemumonitortestutils.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 86883e682b..bb30bed51e 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -124,11 +124,10 @@ qemuMonitorTestAddErrorResponse(qemuMonitorTestPtr test, const char *usermsg) { virBuffer buf = VIR_BUFFER_INITIALIZER; - char *escapemsg = NULL; - char *jsonmsg = NULL; + VIR_AUTOFREE(char *) escapemsg = NULL; + VIR_AUTOFREE(char *) jsonmsg = NULL; const char *monmsg = NULL; char *tmp; - int ret = -1; if (!usermsg) usermsg = "unexpected command"; @@ -136,7 +135,7 @@ qemuMonitorTestAddErrorResponse(qemuMonitorTestPtr test, if (test->json || test->agent) { virBufferEscape(&buf, '\\', "\"", "%s", usermsg); if (virBufferCheckError(&buf) < 0) - goto error; + return -1; escapemsg = virBufferContentAndReset(&buf); /* replace newline/carriage return with space */ @@ -153,19 +152,14 @@ qemuMonitorTestAddErrorResponse(qemuMonitorTestPtr test, " { \"desc\": \"%s\", " " \"class\": \"UnexpectedCommand\" } }", escapemsg) < 0) - goto error; + return -1; monmsg = jsonmsg; } else { monmsg = usermsg; } - ret = qemuMonitorTestAddResponse(test, monmsg); - - error: - VIR_FREE(escapemsg); - VIR_FREE(jsonmsg); - return ret; + return qemuMonitorTestAddResponse(test, monmsg); } -- 2.20.1

On Sat, Jun 15, 2019 at 13:54:02 +0200, Ján Tomko wrote: The check for test->agent is necessary, since we don't know how to validate the agent monitor schema.

On Sat, Jun 15, 2019 at 13:54:03 +0200, Ján Tomko wrote: The agent monitor is based on JSON though. Could you elaborate why you did this change?

We removed testing for text monitor some time ago, remove the remains from the test infrastructure. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- tests/qemumonitortestutils.c | 125 ++++++++++++----------------------- 1 file changed, 44 insertions(+), 81 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index a4fe397be6..f1104e0ff9 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -126,40 +126,33 @@ qemuMonitorTestAddErrorResponse(qemuMonitorTestPtr test, virBuffer buf = VIR_BUFFER_INITIALIZER; VIR_AUTOFREE(char *) escapemsg = NULL; VIR_AUTOFREE(char *) jsonmsg = NULL; - const char *monmsg = NULL; char *tmp; if (!usermsg) usermsg = "unexpected command"; - if (test->json || test->agent) { - virBufferEscape(&buf, '\\', "\"", "%s", usermsg); - if (virBufferCheckError(&buf) < 0) - return -1; - escapemsg = virBufferContentAndReset(&buf); - - /* replace newline/carriage return with space */ - tmp = escapemsg; - while (*tmp) { - if (*tmp == '\r' || *tmp == '\n') - *tmp = ' '; - - tmp++; - } + virBufferEscape(&buf, '\\', "\"", "%s", usermsg); + if (virBufferCheckError(&buf) < 0) + return -1; + escapemsg = virBufferContentAndReset(&buf); - /* format the JSON error message */ - if (virAsprintf(&jsonmsg, "{ \"error\": " - " { \"desc\": \"%s\", " - " \"class\": \"UnexpectedCommand\" } }", - escapemsg) < 0) - return -1; + /* replace newline/carriage return with space */ + tmp = escapemsg; + while (*tmp) { + if (*tmp == '\r' || *tmp == '\n') + *tmp = ' '; - monmsg = jsonmsg; - } else { - monmsg = usermsg; + tmp++; } - return qemuMonitorTestAddResponse(test, monmsg); + /* format the JSON error message */ + if (virAsprintf(&jsonmsg, "{ \"error\": " + " { \"desc\": \"%s\", " + " \"class\": \"UnexpectedCommand\" } }", + escapemsg) < 0) + return -1; + + return qemuMonitorTestAddResponse(test, jsonmsg); } @@ -203,6 +196,7 @@ int ATTRIBUTE_FMT_PRINTF(2, 3) qemuMonitorReportError(qemuMonitorTestPtr test, const char *errmsg, ...) { va_list msgargs; + VIR_AUTOFREE(char *) tmp = NULL; char *msg = NULL; char *jsonmsg = NULL; int ret = -1; @@ -212,20 +206,14 @@ qemuMonitorReportError(qemuMonitorTestPtr test, const char *errmsg, ...) if (virVasprintf(&msg, errmsg, msgargs) < 0) goto cleanup; - if (test->agent || test->json) { - VIR_AUTOFREE(char *) tmp = NULL; - if (!(tmp = qemuMonitorEscapeArg(msg))) - goto cleanup; + if (!(tmp = qemuMonitorEscapeArg(msg))) + goto cleanup; - if (virAsprintf(&jsonmsg, "{ \"error\": " - " { \"desc\": \"%s\", " - " \"class\": \"UnexpectedCommand\" } }", - tmp) < 0) - goto cleanup; - } else { - if (virAsprintf(&jsonmsg, "error: '%s'", msg) < 0) - goto cleanup; - } + if (virAsprintf(&jsonmsg, "{ \"error\": " + " { \"desc\": \"%s\", " + " \"class\": \"UnexpectedCommand\" } }", + tmp) < 0) + goto cleanup; ret = qemuMonitorTestAddResponse(test, jsonmsg); @@ -600,37 +588,19 @@ qemuMonitorTestProcessCommandDefault(qemuMonitorTestPtr test, struct qemuMonitorTestHandlerData *data = item->opaque; virJSONValuePtr val = NULL; virJSONValuePtr cmdargs = NULL; - char *cmdcopy = NULL; const char *cmdname; - char *tmp; int ret = -1; int rc; - if (test->agent || test->json) { - if (!(val = virJSONValueFromString(cmdstr))) - return -1; - - if (!(cmdname = virJSONValueObjectGetString(val, "execute"))) { - ret = qemuMonitorReportError(test, "Missing command name in %s", cmdstr); - goto cleanup; - } - - cmdargs = virJSONValueObjectGet(val, "arguments"); - } else { - if (VIR_STRDUP(cmdcopy, cmdstr) < 0) - return -1; - - cmdname = cmdcopy; + if (!(val = virJSONValueFromString(cmdstr))) + return -1; - if (!(tmp = strchr(cmdcopy, ' '))) { - ret = qemuMonitorReportError(test, - "Cannot find command name in '%s'", - cmdstr); - goto cleanup; - } - *tmp = '\0'; + if (!(cmdname = virJSONValueObjectGetString(val, "execute"))) { + ret = qemuMonitorReportError(test, "Missing command name in %s", cmdstr); + goto cleanup; } + cmdargs = virJSONValueObjectGet(val, "arguments"); if ((rc = qemuMonitorTestProcessCommandDefaultValidate(test, cmdname, cmdargs)) < 0) goto cleanup; @@ -646,7 +616,6 @@ qemuMonitorTestProcessCommandDefault(qemuMonitorTestPtr test, ret = qemuMonitorTestAddResponse(test, data->response); cleanup: - VIR_FREE(cmdcopy); virJSONValueFree(val); return ret; } @@ -689,23 +658,21 @@ qemuMonitorTestProcessCommandVerbatim(qemuMonitorTestPtr test, int rc; /* JSON strings will be reformatted to simplify checking */ - if (test->json || test->agent) { - if (!(json = virJSONValueFromString(cmdstr)) || - !(reformatted = virJSONValueToString(json, false))) - return -1; + if (!(json = virJSONValueFromString(cmdstr)) || + !(reformatted = virJSONValueToString(json, false))) + return -1; - cmdstr = reformatted; + cmdstr = reformatted; - /* in this case we do a best-effort schema check if we can find the command */ - if ((cmdname = virJSONValueObjectGetString(json, "execute"))) { - cmdargs = virJSONValueObjectGet(json, "arguments"); + /* in this case we do a best-effort schema check if we can find the command */ + if ((cmdname = virJSONValueObjectGetString(json, "execute"))) { + cmdargs = virJSONValueObjectGet(json, "arguments"); - if ((rc = qemuMonitorTestProcessCommandDefaultValidate(test, cmdname, cmdargs)) < 0) - return -1; + if ((rc = qemuMonitorTestProcessCommandDefaultValidate(test, cmdname, cmdargs)) < 0) + return -1; - if (rc == 1) - return 0; - } + if (rc == 1) + return 0; } if (STREQ(data->command_name, cmdstr)) { @@ -756,11 +723,7 @@ qemuMonitorTestAddItemVerbatim(qemuMonitorTestPtr test, VIR_STRDUP(data->cmderr, cmderr) < 0) goto error; - if (test->json || test->agent) - data->command_name = virJSONStringReformat(command, false); - else - ignore_value(VIR_STRDUP(data->command_name, command)); - + data->command_name = virJSONStringReformat(command, false); if (!data->command_name) goto error; -- 2.20.1

Use VIR_AUTOPTR and get rid of the 'ret' variable. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- tests/qemumonitortestutils.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 313ecb01fd..ca678e9bd5 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -584,38 +584,29 @@ qemuMonitorTestProcessCommandDefault(qemuMonitorTestPtr test, const char *cmdstr) { struct qemuMonitorTestHandlerData *data = item->opaque; - virJSONValuePtr val = NULL; + VIR_AUTOPTR(virJSONValue) val = NULL; virJSONValuePtr cmdargs = NULL; const char *cmdname; - int ret = -1; int rc; if (!(val = virJSONValueFromString(cmdstr))) return -1; - if (!(cmdname = virJSONValueObjectGetString(val, "execute"))) { - ret = qemuMonitorReportError(test, "Missing command name in %s", cmdstr); - goto cleanup; - } + if (!(cmdname = virJSONValueObjectGetString(val, "execute"))) + return qemuMonitorReportError(test, "Missing command name in %s", cmdstr); cmdargs = virJSONValueObjectGet(val, "arguments"); if ((rc = qemuMonitorTestProcessCommandDefaultValidate(test, cmdname, cmdargs)) < 0) - goto cleanup; - - if (rc == 1) { - ret = 0; - goto cleanup; - } + return -1; + if (rc == 1) + return 0; - if (data->command_name && STRNEQ(data->command_name, cmdname)) - ret = qemuMonitorTestAddInvalidCommandResponse(test, data->command_name, + if (data->command_name && STRNEQ(data->command_name, cmdname)) { + return qemuMonitorTestAddInvalidCommandResponse(test, data->command_name, cmdname); - else - ret = qemuMonitorTestAddResponse(test, data->response); - - cleanup: - virJSONValueFree(val); - return ret; + } else { + return qemuMonitorTestAddResponse(test, data->response); + } } -- 2.20.1

Use VIR_AUTO* for cleanup and virBufferCurrentContent instead of virBufferContentAndReset. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- tests/qemumonitortestutils.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index ca678e9bd5..6be4de8059 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -524,11 +524,10 @@ qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test, const char *cmdname, virJSONValuePtr args) { - virBuffer debug = VIR_BUFFER_INITIALIZER; + VIR_AUTOCLEAN(virBuffer) debug = VIR_BUFFER_INITIALIZER; virJSONValuePtr schemaroot; - virJSONValuePtr emptyargs = NULL; - char *schemapath = NULL; - int ret = -1; + VIR_AUTOPTR(virJSONValue) emptyargs = NULL; + VIR_AUTOFREE(char *) schemapath = NULL; if (!test->qapischema) return 0; @@ -538,43 +537,34 @@ qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test, return 0; if (virAsprintf(&schemapath, "%s/arg-type", cmdname) < 0) - goto cleanup; + return -1; if (virQEMUQAPISchemaPathGet(schemapath, test->qapischema, &schemaroot) < 0 || !schemaroot) { if (qemuMonitorReportError(test, "command '%s' not found in QAPI schema", cmdname) == 0) - ret = 1; - goto cleanup; + return 1; + return -1; } if (!args) { if (!(emptyargs = virJSONValueNewObject())) - goto cleanup; + return -1; args = emptyargs; } if (testQEMUSchemaValidate(args, schemaroot, test->qapischema, &debug) < 0) { - char *debugmsg = virBufferContentAndReset(&debug); if (qemuMonitorReportError(test, "failed to validate arguments of '%s' " "against QAPI schema: %s", - cmdname, debugmsg) == 0) - ret = 1; - - VIR_FREE(debugmsg); - goto cleanup; + cmdname, virBufferCurrentContent(&debug)) == 0) + return 1; + return -1; } - ret = 0; - - cleanup: - virBufferFreeAndReset(&debug); - virJSONValueFree(emptyargs); - VIR_FREE(schemapath); - return ret; + return 0; } -- 2.20.1

Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- tests/qemumonitortestutils.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index c053712573..c14ffb23b9 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -174,17 +174,13 @@ qemuMonitorTestAddInvalidCommandResponse(qemuMonitorTestPtr test, const char *expectedcommand, const char *actualcommand) { - char *msg; - int ret; + VIR_AUTOFREE(char *) msg = NULL; if (virAsprintf(&msg, "expected command '%s' got '%s'", expectedcommand, actualcommand) < 0) return -1; - ret = qemuMonitorTestAddErrorResponse(test, msg); - - VIR_FREE(msg); - return ret; + return qemuMonitorTestAddErrorResponse(test, msg); } -- 2.20.1