On 12/16/2014 06:53 PM, Eric Blake wrote:
On 12/16/2014 06:17 AM, John Ferlan wrote:
Ran the series through my local Coverity checker.... found one issue....
- format, NULL))) + if (format == VIR_STORAGE_FILE_RAW) { + disk->src->capacity = disk->src->physical; + } else if (!(meta = virStorageFileGetMetadataFromBuf(disk->src->path, buf, + len, format, NULL))) { goto endjob; - if (meta->capacity) - disk->src->capacity = meta->capacity; + disk->src->capacity = meta->capacity ? meta->capacity + : disk->src->physical;
We'll never get to this line because of the goto endjob (which gets propagated in next patch as goto cleanup).
Here's what I re-wrote this to:
if (format == VIR_STORAGE_FILE_RAW) src->capacity = src->physical; else if ((meta = virStorageFileGetMetadataFromBuf(disk->src->path, buf, len, format, NULL))) src->capacity = meta->capacity ? meta->capacity : src->physical; else goto endjob;
I didn't see an explicit ACK on this patch, but as 7/12 is the same material just refactored, I'll go ahead and push the corrected form of this at the same time as that one.
Coverity complained this morning because there's a call to virStorageFileGetMetadataFromBuf in the block just before this: (18) Event alloc_fn: Storage is returned from allocation function "virStorageFileGetMetadataFromBuf". [details] (19) Event var_assign: Assigning: "meta" = storage returned from "virStorageFileGetMetadataFromBuf(src->path, buf, len, format, NULL)". (20) Event cond_false: Condition "!(meta = virStorageFileGetMetadataFromBuf(src->path, buf, len, format, NULL))", taking false branch Also see events: [overwrite_var] 11119 if (!(meta = virStorageFileGetMetadataFromBuf(src->path, buf, len, 11120 format, NULL))) (21) Event if_end: End of if statement 11121 goto cleanup; (22) Event cond_false: Condition "format == VIR_STORAGE_FILE_RAW", taking false branch 11122 if (format == VIR_STORAGE_FILE_RAW) 11123 src->capacity = src->physical; (23) Event overwrite_var: Overwriting "meta" in "meta = virStorageFileGetMetadataFromBuf(src->path, buf, len, format, NULL)" leaks the storage that "meta" points to. Also see events: [alloc_fn][var_assign] 11124 else if ((meta = virStorageFileGetMetadataFromBuf(src->path, buf, 11125 len, format, NULL))) 11126 src->capacity = meta->capacity ? meta->capacity : src->physical; 11127 else 11128 goto cleanup; 11129 John