[libvirt] [PATCH 00/12] tests: remove text monitor testing infrastructure
Ján Tomko (12): tests: assume JSON monitor in qemuMonitorTestNewSimple tests: always assume JSON in qemuMonitorTestNew tests: qemuMonitorTestAddErrorResponse: use VIR_AUTOFREE tests: qemuMonitorTestProcessCommandDefaultValidate: simplify condition tests: assume JSON in qemuMonitorTestIO tests: qemuMonitorReportError: use tmp variable properly tests: remove text monitor testing infrastructure tests: qemuMonitorReportError: use VIR_AUTOFREE tests: refactor qemuMonitorTestProcessCommandDefault tests: refactor qemuMonitorTestProcessCommandDefaultValidate tests: qemuMonitorTestAddUnexpectedErrorResponse: VIR_AUTOFREE tests: qemuMonitorTestAddInvalidCommandResponse: VIR_AUTOFREE tests/qemuhotplugtest.c | 2 +- tests/qemumonitortestutils.c | 235 ++++++++++++----------------------- tests/qemumonitortestutils.h | 9 +- 3 files changed, 84 insertions(+), 162 deletions(-) -- 2.20.1
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@redhat.com> --- 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
Now that all the callers call qemuMonitorTestNew with json=true, remove the argument and always assume JSON. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuhotplugtest.c | 2 +- tests/qemumonitortestutils.c | 15 ++++++--------- tests/qemumonitortestutils.h | 7 +++---- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index bfbf32baa4..b659565a48 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -280,7 +280,7 @@ testQemuHotplug(const void *data) /* Now is the best time to feed the spoofed monitor with predefined * replies. */ - if (!(test_mon = qemuMonitorTestNew(true, driver.xmlopt, vm, &driver, + if (!(test_mon = qemuMonitorTestNew(driver.xmlopt, vm, &driver, NULL, NULL))) goto cleanup; diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 03261d649e..86883e682b 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -1241,11 +1241,8 @@ qemuMonitorCommonTestInit(qemuMonitorTestPtr test) "}" /* We skip the normal handshake reply of "{\"execute\":\"qmp_capabilities\"}" */ -#define QEMU_TEXT_GREETING "QEMU 1.0,1 monitor - type 'help' for more information" - qemuMonitorTestPtr -qemuMonitorTestNew(bool json, - virDomainXMLOptionPtr xmlopt, +qemuMonitorTestNew(virDomainXMLOptionPtr xmlopt, virDomainObjPtr vm, virQEMUDriverPtr driver, const char *greeting, @@ -1259,11 +1256,11 @@ qemuMonitorTestNew(bool json, if (!(test = qemuMonitorCommonTestNew(xmlopt, vm, &src))) goto error; - test->json = json; + test->json = true; test->qapischema = schema; if (!(test->mon = qemuMonitorOpen(test->vm, &src, - json, + true, true, 0, &qemuMonitorTestCallbacks, @@ -1273,7 +1270,7 @@ qemuMonitorTestNew(bool json, virObjectLock(test->mon); if (!greeting) - greeting = json ? QEMU_JSON_GREETING : QEMU_TEXT_GREETING; + greeting = QEMU_JSON_GREETING; if (qemuMonitorTestAddResponse(test, greeting) < 0) goto error; @@ -1340,7 +1337,7 @@ qemuMonitorTestNewFromFile(const char *fileName, goto error; } else { /* Create new mocked monitor with our greeting */ - if (!(test = qemuMonitorTestNew(true, xmlopt, NULL, NULL, + if (!(test = qemuMonitorTestNew(xmlopt, NULL, NULL, singleReply, NULL))) goto error; } @@ -1425,7 +1422,7 @@ qemuMonitorTestNewFromFileFull(const char *fileName, if (virTestLoadFile(fileName, &jsonstr) < 0) return NULL; - if (!(ret = qemuMonitorTestNew(true, driver->xmlopt, vm, driver, NULL, + if (!(ret = qemuMonitorTestNew(driver->xmlopt, vm, driver, NULL, qmpschema))) goto cleanup; diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h index 23192c1223..3e93f9551a 100644 --- a/tests/qemumonitortestutils.h +++ b/tests/qemumonitortestutils.h @@ -75,12 +75,11 @@ int qemuMonitorTestAddItemExpect(qemuMonitorTestPtr test, const char *response); # define qemuMonitorTestNewSimple(xmlopt) \ - qemuMonitorTestNew(true, xmlopt, NULL, NULL, NULL, NULL) + qemuMonitorTestNew(xmlopt, NULL, NULL, NULL, NULL) # define qemuMonitorTestNewSchema(xmlopt, schema) \ - qemuMonitorTestNew(true, xmlopt, NULL, NULL, NULL, schema) + qemuMonitorTestNew(xmlopt, NULL, NULL, NULL, schema) -qemuMonitorTestPtr qemuMonitorTestNew(bool json, - virDomainXMLOptionPtr xmlopt, +qemuMonitorTestPtr qemuMonitorTestNew(virDomainXMLOptionPtr xmlopt, virDomainObjPtr vm, virQEMUDriverPtr driver, const char *greeting, -- 2.20.1
Use VIR_AUTOFREE. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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
This function is skipped for non-JSON monitor (which never happens now) and for agent connections (which we currently do not test). Only check for qapischema to simplify the condition. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemumonitortestutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index bb30bed51e..cf4fec8873 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -546,7 +546,7 @@ qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test, char *schemapath = NULL; int ret = -1; - if (!test->qapischema || !test->json || test->agent) + if (!test->qapischema) return 0; /* 'device_add' needs to be skipped as it does not have fully defined schema */ -- 2.20.1
On Sat, Jun 15, 2019 at 13:54:02 +0200, Ján Tomko wrote:
This function is skipped for non-JSON monitor (which never happens now) and for agent connections (which we currently do not test).
Only check for qapischema to simplify the condition.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemumonitortestutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index bb30bed51e..cf4fec8873 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -546,7 +546,7 @@ qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test, char *schemapath = NULL; int ret = -1;
- if (!test->qapischema || !test->json || test->agent) + if (!test->qapischema)
The check for test->agent is necessary, since we don't know how to validate the agent monitor schema.
Only strip the carriage return for agent monitor, now that we no longer support text monitor. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemumonitortestutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index cf4fec8873..e2144923f0 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -320,7 +320,7 @@ qemuMonitorTestIO(virNetSocketPtr sock, */ t1 = test->incoming; while ((t2 = strstr(t1, "\n")) || - (!test->json && (t2 = strstr(t1, "\r")))) { + (test->agent && (t2 = strstr(t1, "\r")))) { *t2 = '\0'; if (qemuMonitorTestProcessCommand(test, t1) < 0) { -- 2.20.1
On Sat, Jun 15, 2019 at 13:54:03 +0200, Ján Tomko wrote:
Only strip the carriage return for agent monitor, now that we no longer support text monitor.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemumonitortestutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index cf4fec8873..e2144923f0 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -320,7 +320,7 @@ qemuMonitorTestIO(virNetSocketPtr sock, */ t1 = test->incoming; while ((t2 = strstr(t1, "\n")) || - (!test->json && (t2 = strstr(t1, "\r")))) { + (test->agent && (t2 = strstr(t1, "\r")))) {
The agent monitor is based on JSON though. Could you elaborate why you did this change?
There is no obvious benefit in putting the escaped message back into msg while tmp holds the original message. Remove the assignment and use 'tmp' directly'. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemumonitortestutils.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index e2144923f0..a4fe397be6 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -213,16 +213,14 @@ qemuMonitorReportError(qemuMonitorTestPtr test, const char *errmsg, ...) goto cleanup; if (test->agent || test->json) { - char *tmp = msg; - msg = qemuMonitorEscapeArg(tmp); - VIR_FREE(tmp); - if (!msg) + VIR_AUTOFREE(char *) tmp = NULL; + if (!(tmp = qemuMonitorEscapeArg(msg))) goto cleanup; if (virAsprintf(&jsonmsg, "{ \"error\": " " { \"desc\": \"%s\", " " \"class\": \"UnexpectedCommand\" } }", - msg) < 0) + tmp) < 0) goto cleanup; } else { if (virAsprintf(&jsonmsg, "error: '%s'", msg) < 0) -- 2.20.1
We removed testing for text monitor some time ago, remove the remains from the test infrastructure. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemumonitortestutils.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index f1104e0ff9..313ecb01fd 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -197,8 +197,8 @@ qemuMonitorReportError(qemuMonitorTestPtr test, const char *errmsg, ...) { va_list msgargs; VIR_AUTOFREE(char *) tmp = NULL; - char *msg = NULL; - char *jsonmsg = NULL; + VIR_AUTOFREE(char *) msg = NULL; + VIR_AUTOFREE(char *) jsonmsg = NULL; int ret = -1; va_start(msgargs, errmsg); @@ -219,8 +219,6 @@ qemuMonitorReportError(qemuMonitorTestPtr test, const char *errmsg, ...) cleanup: va_end(msgargs); - VIR_FREE(msg); - VIR_FREE(jsonmsg); return ret; } -- 2.20.1
Use VIR_AUTOPTR and get rid of the 'ret' variable. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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
On Sat, Jun 15, 2019 at 13:54:07 +0200, Ján Tomko wrote:
Use VIR_AUTOPTR and get rid of the 'ret' variable.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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);
Align this line.
Use VIR_AUTO* for cleanup and virBufferCurrentContent instead of virBufferContentAndReset. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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@redhat.com> --- tests/qemumonitortestutils.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 6be4de8059..c053712573 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -160,16 +160,12 @@ static int qemuMonitorTestAddUnexpectedErrorResponse(qemuMonitorTestPtr test, const char *command) { - char *msg; - int ret; + VIR_AUTOFREE(char *) msg = NULL; if (virAsprintf(&msg, "unexpected command: '%s'", command) < 0) return -1; - ret = qemuMonitorTestAddErrorResponse(test, msg); - - VIR_FREE(msg); - return ret; + return qemuMonitorTestAddErrorResponse(test, msg); } -- 2.20.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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
On Sat, Jun 15, 2019 at 13:53:58 +0200, Ján Tomko wrote:
Ján Tomko (12): tests: assume JSON monitor in qemuMonitorTestNewSimple tests: always assume JSON in qemuMonitorTestNew tests: qemuMonitorTestAddErrorResponse: use VIR_AUTOFREE tests: qemuMonitorTestProcessCommandDefaultValidate: simplify condition tests: assume JSON in qemuMonitorTestIO tests: qemuMonitorReportError: use tmp variable properly tests: remove text monitor testing infrastructure tests: qemuMonitorReportError: use VIR_AUTOFREE tests: refactor qemuMonitorTestProcessCommandDefault tests: refactor qemuMonitorTestProcessCommandDefaultValidate tests: qemuMonitorTestAddUnexpectedErrorResponse: VIR_AUTOFREE tests: qemuMonitorTestAddInvalidCommandResponse: VIR_AUTOFREE
ACK to all the patches I did not comment on.
participants (2)
-
Ján Tomko -
Peter Krempa