On Mon, Aug 13, 2018 at 18:27:06 -0400, John Ferlan wrote:
On 08/01/2018 01:02 PM, Daniel P. Berrangé wrote:
The x86 and ppc impls both duplicate some logic when parsing CPU features. Change the callback signature so that this duplication can be pushed up a level to common code.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/cpu/cpu_map.c | 106 +++++++++++++++--------- src/cpu/cpu_map.h | 22 ++--- src/cpu/cpu_ppc64.c | 112 ++++++------------------- src/cpu/cpu_x86.c | 196 +++++++++++++------------------------------- 4 files changed, 155 insertions(+), 281 deletions(-)
...
+static int x86ModelParse(xmlXPathContextPtr ctxt, - virCPUx86MapPtr map) + const char *name, + void *data) {
[...]
+ if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0) + goto error;
Similar regarding cleanup/error; however, ...
+ + ret = 0; + cleanup:
I ran the changes through Coverity and it complains here because one can goto cleanup if the signature code fails, return -1, but the @model wouldn't be free'd.
Prior to this change it seems failing in that code wasn't necessarily an error, but it would generate the error, not go through the vendor and feature processing, and return a somewhat empty @model. Perhaps a bug in existing code, but uncovered in this refactoring.
Hmm, the gotos in the signature code should be jumping to the error label. I guess the best fix would be a separate patch removing the error label completely. Jirka