[PATCH 06/16] qemuMonitorJSONAttachCharDevGetProps: Modernize construction of JSON objects
Ján Tomko
jtomko at redhat.com
Fri Nov 19 10:31:54 UTC 2021
On a Thursday in 2021, Peter Krempa wrote:
>Use 'virJSONValueObjectAdd' instead of the step-by-step manual JSON
>object building.
>
>Signed-off-by: Peter Krempa <pkrempa at redhat.com>
>---
> src/qemu/qemu_monitor_json.c | 195 ++++++++++++++++++-----------------
> 1 file changed, 99 insertions(+), 96 deletions(-)
>
>diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>index 1ced942161..a9e87de4d3 100644
>--- a/src/qemu/qemu_monitor_json.c
>+++ b/src/qemu/qemu_monitor_json.c
> qemuMonitorJSONAttachCharDevGetProps(const char *chrID,
> const virDomainChrSourceDef *chr)
> {
> g_autoptr(virJSONValue) props = NULL;
>- g_autoptr(virJSONValue) backend = virJSONValueNewObject();
>- g_autoptr(virJSONValue) data = virJSONValueNewObject();
>- g_autoptr(virJSONValue) addr = NULL;
>- const char *backend_type = NULL;
>- const char *host;
>- const char *port;
>- g_autofree char *tlsalias = NULL;
>- bool telnet;
>+ g_autoptr(virJSONValue) backend = NULL;
>+ g_autoptr(virJSONValue) backendData = virJSONValueNewObject();
>+ const char *backendType = NULL;
The backendType and backendData renames could've been done upfront to
simplify this patch, at the cost of changing the same lines multiple
times.
>
> switch ((virDomainChrType)chr->type) {
> case VIR_DOMAIN_CHR_TYPE_NULL:
>- backend_type = "null";
>- break;
>-
> case VIR_DOMAIN_CHR_TYPE_VC:
>- backend_type = "vc";
>- break;
>-
> case VIR_DOMAIN_CHR_TYPE_PTY:
>- backend_type = "pty";
>+ backendType = virDomainChrTypeToString(chr->type);
[...]
>- if (qemuMonitorJSONBuildChrChardevReconnect(data, &chr->data.tcp.reconnect) < 0)
>- return NULL;
>- break;
>
>- case VIR_DOMAIN_CHR_TYPE_UDP:
>- backend_type = "udp";
>- host = chr->data.udp.connectHost;
>- if (!host)
>- host = "";
>- addr = qemuMonitorJSONBuildInetSocketAddress(host,
>- chr->data.udp.connectService);
>- if (!addr ||
>- virJSONValueObjectAppend(data, "remote", &addr) < 0)
>- return NULL;
>+ if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_YES)
>+ reconnect = chr->data.tcp.reconnect.timeout;
>+ else if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_NO)
>+ reconnect = 0;
Before, qemuMonitorJSONBuildChrChardevReconnect only formatted the
timeout value if enabled was set to BOOL_YES.
[A] Please split out the == BOOL_NO additions that unify it with what we do
in the cold start commandline generator.
>+ } else {
>+ if (chr->data.nix.listen) {
>+ server = true;
>+ waitval = VIR_TRISTATE_BOOL_NO;
>+ }
>
[...]
>@@ -6748,15 +6748,18 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID,
> return NULL;
> }
>
>- if (chr->logfile &&
>- virJSONValueObjectAdd(&data,
>- "s:logfile", chr->logfile,
>- "T:logappend", chr->logappend,
>- NULL) < 0)
>- return NULL;
>+ if (chr->logfile) {
>+ if (virJSONValueObjectAdd(&backendData,
>+ "S:logfile", chr->logfile,
>+ "T:logappend", chr->logappend,
>+ NULL) < 0)
>+ return NULL;
>+ }
Apart from the style change [B] There's no need to change 's' to 'S'
here.
>
>- if (virJSONValueObjectAppendString(backend, "type", backend_type) < 0 ||
>- virJSONValueObjectAppend(backend, "data", &data) < 0)
>+ if (virJSONValueObjectAdd(&backend,
>+ "s:type", backendType,
>+ "A:data", &backendData,
>+ NULL) < 0)
> return NULL;
>
With [A][B] addressed,
Reviewed-by: Ján Tomko <jtomko at redhat.com>
Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 484 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20211119/28b6b7ee/attachment-0001.sig>
More information about the libvir-list
mailing list