Re: [libvirt] [PATCH] conf: rework code around 'append' attribute to make coverity happy
On 12/26/2015 12:18 PM, Dmitry Mishin wrote:
Signed-off-by: Dmitry Mishin <dim(a)virtuozzo.com>
---
src/conf/domain_conf.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f210c0b..8895e10 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1723,10 +1723,11 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
switch (src->type) {
case VIR_DOMAIN_CHR_TYPE_FILE:
- dest->data.file.append = src->data.file.append;
case VIR_DOMAIN_CHR_TYPE_PTY:
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_PIPE:
+ if (src->type == VIR_DOMAIN_CHR_TYPE_FILE)
+ dest->data.file.append = src->data.file.append;
if (VIR_STRDUP(dest->data.file.path, src->data.file.path) < 0)
return -1;
break;
@@ -9386,12 +9387,12 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
switch ((virDomainChrType) def->type) {
case VIR_DOMAIN_CHR_TYPE_FILE:
- if (!append)
- append = virXMLPropString(cur, "append");
case VIR_DOMAIN_CHR_TYPE_PTY:
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_PIPE:
case VIR_DOMAIN_CHR_TYPE_UNIX:
+ if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
+ append = virXMLPropString(cur, "append");
/* PTY path is only parsed from live xml. */
if (!path &&
(def->type != VIR_DOMAIN_CHR_TYPE_PTY ||
@@ -9476,15 +9477,15 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
break;
case VIR_DOMAIN_CHR_TYPE_FILE:
+ case VIR_DOMAIN_CHR_TYPE_PTY:
+ case VIR_DOMAIN_CHR_TYPE_DEV:
+ case VIR_DOMAIN_CHR_TYPE_PIPE:
if (append &&
Wouldn't this last one be :
if (append && def->type == VIR_DOMAIN_CHR_TYPE_FILE &&
(def->data.file.append =
virTristateSwitchTypeFromString(append)) <= 0) {
NB:
Other uses of def->data.file.append could have used the
virTristateSwitch constants (VIR_TRISTATE_SWITCH_ABSENT,
VIR_TRISTATE_SWITCH_ON, and VIR_TRISTATE_SWITCH_OFF).
That is rather than "if (dev->data.file.append)", it could be "if
(dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT). It's just
"safer" that way. Does adding ",append=off" make much sense? Or do
you
expect some day in the future that append=on becomes the "default"?
You could have added an example where append='off' - it shouldn't
matter, but it is a legal option and the right XML should be generated
and the right .args would have append=off added to the command line.
John
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid append attribute value '%s'"),
append);
goto error;
}
- case VIR_DOMAIN_CHR_TYPE_PTY:
- case VIR_DOMAIN_CHR_TYPE_DEV:
- case VIR_DOMAIN_CHR_TYPE_PIPE:
if (!path &&
def->type != VIR_DOMAIN_CHR_TYPE_PTY) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",