On Fri, Jan 31, 2014 at 05:08:32PM +0100, Martin Kletzander wrote:
> Some cleanups around serial chardevs and miscellaneous things I've
> found inconsistent or not very clean.
A few comments below, I'd tend to split at least the bigger bits in
separate patches to avoid a bunch of unrelated changes in a single commit
Christophe
>
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 43 ++++++++++++++++++++++++++-----------------
> src/qemu/qemu_capabilities.c | 4 ++--
> src/qemu/qemu_command.c | 10 ++++++++--
> 3 files changed, 36 insertions(+), 21 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 28e24f9..fa1ecb5 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1559,7 +1559,7 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
> if (tgt->type != src->type)
> return false;
>
> - switch (src->type) {
> + switch ((enum virDomainChrType)src->type) {
Not sure this one improves things, the type of the enum is in the
namespacing of the values anyway (VIR_DOMAIN_CHR_TYPE_*), and it's only
done for a minority of the switch() calls in domain_conf.c
> case VIR_DOMAIN_CHR_TYPE_PTY:
> case VIR_DOMAIN_CHR_TYPE_DEV:
> case VIR_DOMAIN_CHR_TYPE_FILE:
> @@ -1583,16 +1583,22 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
> STREQ_NULLABLE(src->data.nix.path, tgt->data.nix.path);
> break;
>
> + case VIR_DOMAIN_CHR_TYPE_NULL:
> case VIR_DOMAIN_CHR_TYPE_VC:
> case VIR_DOMAIN_CHR_TYPE_STDIO:
> case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
> + case VIR_DOMAIN_CHR_TYPE_LAST:
> /* nada */
> return true;
> }
>
> - /* This should happen only on new,
> - * yet unhandled type */
> + /* Even though gcc is able to detect that all possible values are
> + * handled in the switch above, it is not capable of realizing
> + * there isn't any possibility of escaping that switch without
> + * return. So for the sake of clean compilation, there has to be
> + * a return here */
>
> + /* coverity[dead_error_begin] */
> return false;
> }
>
> @@ -6415,7 +6421,7 @@ error:
> }
>
> #define NET_MODEL_CHARS \
> - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_-"
> + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-"
>
> /* Parse the XML definition for a network interface
> * @param node XML nodeset to parse for net definition
> @@ -7182,11 +7188,11 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> }
>
> switch ((enum virDomainChrType) def->type) {
> - case VIR_DOMAIN_CHR_TYPE_LAST:
> case VIR_DOMAIN_CHR_TYPE_NULL:
> + case VIR_DOMAIN_CHR_TYPE_VC:
> case VIR_DOMAIN_CHR_TYPE_STDIO:
> case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
> - case VIR_DOMAIN_CHR_TYPE_VC:
> + case VIR_DOMAIN_CHR_TYPE_LAST:
> break;
>
> case VIR_DOMAIN_CHR_TYPE_PTY:
> @@ -15573,11 +15579,13 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
> }
> virBufferAddLit(buf, ">\n");
>
> - switch (def->type) {
> + virBufferAdjustIndent(buf, 6);
> + switch ((enum virDomainChrType)def->type) {
> case VIR_DOMAIN_CHR_TYPE_NULL:
> case VIR_DOMAIN_CHR_TYPE_VC:
> case VIR_DOMAIN_CHR_TYPE_STDIO:
> case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
> + case VIR_DOMAIN_CHR_TYPE_LAST:
> /* nada */
> break;
>
> @@ -15588,7 +15596,7 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
> if (def->type != VIR_DOMAIN_CHR_TYPE_PTY ||
> (def->data.file.path &&
> !(flags & VIR_DOMAIN_XML_INACTIVE))) {
> - virBufferEscapeString(buf, " <source path='%s'/>\n",
> + virBufferEscapeString(buf, "<source path='%s'/>\n",
> def->data.file.path);
> }
> break;
> @@ -15597,53 +15605,54 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
> if (def->data.udp.bindService &&
> def->data.udp.bindHost) {
> virBufferAsprintf(buf,
> - " <source mode='bind' host='%s' "
> + "<source mode='bind' host='%s' "
> "service='%s'/>\n",
> def->data.udp.bindHost,
> def->data.udp.bindService);
> } else if (def->data.udp.bindHost) {
> - virBufferAsprintf(buf, " <source mode='bind' host='%s'/>\n",
> + virBufferAsprintf(buf, "<source mode='bind' host='%s'/>\n",
> def->data.udp.bindHost);
> } else if (def->data.udp.bindService) {
> - virBufferAsprintf(buf, " <source mode='bind' service='%s'/>\n",
> + virBufferAsprintf(buf, "<source mode='bind' service='%s'/>\n",
> def->data.udp.bindService);
> }
>
> if (def->data.udp.connectService &&
> def->data.udp.connectHost) {
> virBufferAsprintf(buf,
> - " <source mode='connect' host='%s' "
> + "<source mode='connect' host='%s' "
> "service='%s'/>\n",
> def->data.udp.connectHost,
> def->data.udp.connectService);
> } else if (def->data.udp.connectHost) {
> - virBufferAsprintf(buf, " <source mode='connect' host='%s'/>\n",
> + virBufferAsprintf(buf, "<source mode='connect' host='%s'/>\n",
> def->data.udp.connectHost);
> } else if (def->data.udp.connectService) {
> virBufferAsprintf(buf,
> - " <source mode='connect' service='%s'/>\n",
> + "<source mode='connect' service='%s'/>\n",
> def->data.udp.connectService);
> }
> break;
>
> case VIR_DOMAIN_CHR_TYPE_TCP:
> virBufferAsprintf(buf,
> - " <source mode='%s' host='%s' service='%s'/>\n",
> + "<source mode='%s' host='%s' service='%s'/>\n",
> def->data.tcp.listen ? "bind" : "connect",
> def->data.tcp.host,
> def->data.tcp.service);
> - virBufferAsprintf(buf, " <protocol type='%s'/>\n",
> + virBufferAsprintf(buf, "<protocol type='%s'/>\n",
> virDomainChrTcpProtocolTypeToString(
> def->data.tcp.protocol));
> break;
>
> case VIR_DOMAIN_CHR_TYPE_UNIX:
> - virBufferAsprintf(buf, " <source mode='%s'",
> + virBufferAsprintf(buf, "<source mode='%s'",
> def->data.nix.listen ? "bind" : "connect");
> virBufferEscapeString(buf, " path='%s'", def->data.nix.path);
> virBufferAddLit(buf, "/>\n");
> break;
> }
> + virBufferAdjustIndent(buf, -6);
>
> return 0;
> }
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 8420047..8aec293 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -1,7 +1,7 @@
> /*
> * qemu_capabilities.c: QEMU capabilities generation
> *
> - * Copyright (C) 2006-2013 Red Hat, Inc.
> + * Copyright (C) 2006-2014 Red Hat, Inc.
> * Copyright (C) 2006 Daniel P. Berrange
> *
> * This library is free software; you can redistribute it and/or
> @@ -247,7 +247,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
> "boot-strict", /* 160 */
> "pvpanic",
> "enable-fips",
> - "spice-file-xfer-disable"
> + "spice-file-xfer-disable",
> );
>
> struct _virQEMUCaps {
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 96b8825..2db745a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1,7 +1,7 @@
> /*
> * qemu_command.c: QEMU command generation
> *
> - * Copyright (C) 2006-2013 Red Hat, Inc.
> + * Copyright (C) 2006-2014 Red Hat, Inc.
> * Copyright (C) 2006 Daniel P. Berrange
> *
> * This library is free software; you can redistribute it and/or
> @@ -6004,7 +6004,7 @@ qemuBuildChrArgStr(virDomainChrSourceDefPtr dev, const char *prefix)
> if (prefix)
> virBufferAdd(&buf, prefix, strlen(prefix));
>
> - switch (dev->type) {
> + switch ((enum virDomainChrType)dev->type) {
> case VIR_DOMAIN_CHR_TYPE_NULL:
> virBufferAddLit(&buf, "null");
> break;
> @@ -6071,6 +6071,12 @@ qemuBuildChrArgStr(virDomainChrSourceDefPtr dev, const char *prefix)
> dev->data.nix.path,
> dev->data.nix.listen ? ",server,nowait" : "");
> break;
> +
> + case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
> + /* spicevmc doesn't have any '-serial' compatible option */
This is misleading as this function is also called for -monitor, and
-parallel.
> + case VIR_DOMAIN_CHR_TYPE_LAST:
> + /* coverity[dead_error_begin] */
> + break;
> }
>
> if (virBufferError(&buf)) {
> --
> 1.8.5.3
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
pgpbWPNEeCcIu.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list