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...>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to