5:40 a.m.
On 06/17/2013 01:56 PM, james robson wrote:
<...snip...>
> diff --git a/src/util/virnetdevopenvswitch.c
b/src/util/virnetdevopenvswitch.c
> index 2aee445..47e6027 100644
> --- a/src/util/virnetdevopenvswitch.c
> +++ b/src/util/virnetdevopenvswitch.c
> @@ -109,8 +109,22 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char
*ifname,
> virCommandAddArgList(cmd, "--timeout=5", "--",
"--may-exist", "add-port",
> brname, ifname, NULL);
>
> - if (virBufferUse(&buf) != 0)
> + if (virBufferUse(&buf) != 0) {
> + switch (virtVlan->nativeMode) {
> + case VIR_NATIVE_VLAN_MODE_TAGGED:
> + virCommandAddArg(cmd, "vlan_mode=native-tagged");
> + virCommandAddArgFormat(cmd, "tag=%d",
virtVlan->nativeTag);
> + break;
> + case VIR_NATIVE_VLAN_MODE_UNTAGGED:
> + virCommandAddArg(cmd, "vlan_mode=native-untagged");
> + virCommandAddArgFormat(cmd, "tag=%d",
virtVlan->nativeTag);
> + break;
> + case VIR_NATIVE_VLAN_MODE_DEFAULT:
> + default:
> + break;
> + }
> virCommandAddArgList(cmd, virBufferCurrentContent(&buf), NULL);
> + }
My overnight Coverity run has found the following problem with above code.
The issue is that at line 84 there's an "if (virtVlan &&" condition;
whereas,
this reference at line 112 doesn't check virtVlan before referencing:
68
(1) Event cond_false: Condition "virAsprintf(&attachedmac_ex_id,
"external-ids:attached-mac=\"%s\"", macaddrstr) < 0", taking
false branch
69 if (virAsprintf(&attachedmac_ex_id,
"external-ids:attached-mac=\"%s\"",
70 macaddrstr) < 0)
71 goto out_of_memory;
(2) Event cond_false: Condition "virAsprintf(&ifaceid_ex_id,
"external-ids:iface-id=\"%s\"", ifuuidstr) < 0", taking false
branch
72 if (virAsprintf(&ifaceid_ex_id,
"external-ids:iface-id=\"%s\"",
73 ifuuidstr) < 0)
74 goto out_of_memory;
(3) Event cond_false: Condition "virAsprintf(&vmid_ex_id,
"external-ids:vm-id=\"%s\"", vmuuidstr) < 0", taking false
branch
75 if (virAsprintf(&vmid_ex_id,
"external-ids:vm-id=\"%s\"",
76 vmuuidstr) < 0)
77 goto out_of_memory;
(4) Event cond_true: Condition "ovsport->profileID[0] != 0", taking true
branch
78 if (ovsport->profileID[0] != '\0') {
(5) Event cond_false: Condition "virAsprintf(&profile_ex_id,
"external-ids:port-profile=\"%s\"", ovsport->profileID) <
0", taking false branch
79 if (virAsprintf(&profile_ex_id,
"external-ids:port-profile=\"%s\"",
80 ovsport->profileID) < 0)
81 goto out_of_memory;
82 }
83
(6) Event cond_false: Condition "virtVlan", taking false branch
(8) Event var_compare_op: Comparing "virtVlan" to null implies that
"virtVlan" might be null.
Also see events: [var_deref_op]
84 if (virtVlan && virtVlan->nTags > 0) {
85
86 /* Trunk port first */
87 if (virtVlan->trunk) {
88 virBufferAddLit(&buf, "trunk=");
89
90 /*
91 * Trunk ports have at least one VLAN. Do the first one
92 * outside the "for" loop so we can put a "," at
the
93 * start of the for loop if there are more than one VLANs
94 * on this trunk port.
95 */
96 virBufferAsprintf(&buf, "%d", virtVlan->tag[i]);
97
98 for (i = 1; i < virtVlan->nTags; i++) {
99 virBufferAddLit(&buf, ",");
100 virBufferAsprintf(&buf, "%d", virtVlan->tag[i]);
101 }
102 } else if (virtVlan->nTags) {
103 virBufferAsprintf(&buf, "tag=%d", virtVlan->tag[0]);
104 }
(7) Event if_end: End of if statement
105 }
106
107 cmd = virCommandNew(OVSVSCTL);
108
109 virCommandAddArgList(cmd, "--timeout=5", "--",
"--may-exist", "add-port",
110 brname, ifname, NULL);
111
(9) Event var_deref_op: Dereferencing null pointer "virtVlan".
Also see events: [var_compare_op]
112 switch (virtVlan->nativeMode) {
113 case VIR_NATIVE_VLAN_MODE_TAGGED:
114 virCommandAddArg(cmd, "vlan_mode=native-tagged");
115 virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
116 break;
117 case VIR_NATIVE_VLAN_MODE_UNTAGGED:
118 virCommandAddArg(cmd, "vlan_mode=native-untagged");
119 virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
120 break;
121 case VIR_NATIVE_VLAN_MODE_DEFAULT:
122 default:
123 break;
124 }
<...snip...>