On 05/23/2013 09:44 AM, Michal Privoznik wrote:
> On 23.05.2013 15:04, John Ferlan wrote:
>> On 05/20/2013 01:55 PM, Michal Privoznik wrote:
>>> ---
>> [...snip...]
>>
>>> diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
>>> index 2509c0d..ac5f7ed 100644
>>> --- a/src/conf/nwfilter_params.c
>>> +++ b/src/conf/nwfilter_params.c
>>> @@ -79,19 +79,15 @@ virNWFilterVarValueCopy(const virNWFilterVarValuePtr val)
>>>
>>> switch (res->valType) {
>>> case NWFILTER_VALUE_TYPE_SIMPLE:
>>> - if (val->u.simple.value) {
>>> - res->u.simple.value = strdup(val->u.simple.value);
>>> - if (!res->u.simple.value)
>>> - goto err_exit;
>>> - }
>>> + if (VIR_STRDUP(res->u.simple.value, val->u.simple.value) < 0)
>>> + goto err_exit;
>>> break;
>>> case NWFILTER_VALUE_TYPE_ARRAY:
>>> if (VIR_ALLOC_N(res->u.array.values, val->u.array.nValues) < 0)
>>> goto err_exit;
>>> res->u.array.nValues = val->u.array.nValues;
>>> for (i = 0; i < val->u.array.nValues; i++) {
>>> - str = strdup(val->u.array.values[i]);
>>> - if (!str)
>>> + if (VIR_STRDUP(str, val->u.array.values[i]) < 0)
>>> goto err_exit;
>>> res->u.array.values[i] = str;
>>> }
>>> @@ -133,12 +129,10 @@ virNWFilterVarValueCreateSimple(char *value)
>>> virNWFilterVarValuePtr
>>> virNWFilterVarValueCreateSimpleCopyValue(const char *value)
>>> {
>>> - char *val = strdup(value);
>>> + char *val;
>>>
>>> - if (!val) {
>>> - virReportOOMError();
>>> + if (VIR_STRDUP(val, value) < 0)
>>> return NULL;
>>> - }
>>> return virNWFilterVarValueCreateSimple(val);
>>> }
>>>
>>> @@ -654,17 +648,15 @@ virNWFilterHashTablePut(virNWFilterHashTablePtr table,
>>> {
>>> if (!virHashLookup(table->hashTable, name)) {
>>> if (copyName) {
>>> - name = strdup(name);
>>> - if (!name) {
>>> - virReportOOMError();
>>> + char *newName;
>>> + if (VIR_STRDUP(newName, name) < 0)
>>> return -1;
>>> - }
>>>
>>> if (VIR_REALLOC_N(table->names, table->nNames + 1) < 0) {
>>> VIR_FREE(name);
>>> return -1;
>>> }
>>> - table->names[table->nNames++] = (char *)name;
>>> + table->names[table->nNames++] = newName;
>>> }
>>
>> Coverity complains (see below too)
>>
>> 648 {
>>
>> (1) Event cond_true: Condition "!virHashLookup(table->hashTable, name)", taking true branch
>>
>> 649 if (!virHashLookup(table->hashTable, name)) {
>>
>> (2) Event cond_true: Condition "copyName", taking true branch
>>
>> 650 if (copyName) {
>> 651 char *newName;
>>
>> (3) Event cond_false: Condition "virStrdup(&newName, name, true /* 1 */, VIR_FROM_NWFILTER, "conf/nwfilter_params.c", <anonymous>, 652) < 0", taking false branch
>>
>> 652 if (VIR_STRDUP(newName, name) < 0)
>> 653 return -1;
>> 654
>>
>> (5) Event cond_false: Condition "virReallocN(&table->names, 8UL /* sizeof (*table->names) */, table->nNames + 1) < 0", taking false branch
>>
>> 655 if (VIR_REALLOC_N(table->names, table->nNames + 1) < 0) {
>> 656 VIR_FREE(name);
>> 657 return -1;
>>
>> (6) Event if_end: End of if statement
>>
>> 658 }
>> 659 table->names[table->nNames++] = newName;
>> 660 }
>> 661
>>
>> (7) Event cond_true: Condition "virHashAddEntry(table->hashTable, name, val) < 0", taking true branch
>>
>> 662 if (virHashAddEntry(table->hashTable, name, val) < 0) {
>>
>> (8) Event cond_true: Condition "copyName", taking true branch
>>
>> 663 if (copyName) {
>>
>> (9) Event freed_arg: "virFree(void *)" frees parameter "name". [details]
>>
>> 664 VIR_FREE(name);
>> 665 table->nNames--;
>> 666 }
>> 667 return -1;
>> 668 }
>>
>>
>> In particular, the "VIR_FREE(name)" causes a problem in one of the callers,
>> see below. So I think the fix is to dump "newNames" up one level, then change:
>>
>> 664 VIR_FREE(name);
>> 665 table->nNames--;
>>
>> to
>>
>> 664 VIR_FREE(newName);
>> 665 table->nNames--;
>>
>>
>> John
>
> Right, it took me a while to parse the coverity output, and here's the
> part of the original code that confused me:
>
> if (!virHashLookup(table->hashTable, name)) {
> if (copyName) {
> name = strdup(name);
>
>
>
Taking advantage that when returned to the caller name is what it was...
The following seems right (that is use newName instead of name locally):
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index ac5f7ed..44b9f05 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -647,13 +647,13 @@ virNWFilterHashTablePut(virNWFilterHashTablePtr table,
int copyName)
{
if (!virHashLookup(table->hashTable, name)) {
+ char *newName = NULL;
if (copyName) {
- char *newName;
if (VIR_STRDUP(newName, name) < 0)
return -1;
if (VIR_REALLOC_N(table->names, table->nNames + 1) < 0) {
- VIR_FREE(name);
+ VIR_FREE(newName);
return -1;
}
table->names[table->nNames++] = newName;
@@ -661,7 +661,7 @@ virNWFilterHashTablePut(virNWFilterHashTablePtr table,
if (virHashAddEntry(table->hashTable, name, val) < 0) {
if (copyName) {
- VIR_FREE(name);
+ VIR_FREE(newName);
table->nNames--;
}
return -1;
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list