[PATCH] cpu_ppc64: compare CPU function is ignoring return value

Julio Faracco posted 1 patch 3 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201027140540.15585-1-jcfaracco@gmail.com
There is a newer version of this series
src/cpu/cpu_ppc64.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] cpu_ppc64: compare CPU function is ignoring return value
Posted by Julio Faracco 3 years, 6 months ago
Function to compare CPU on 64-bit PowerPC is ignoring the flag to avoid 
failure in case of CPUs (host and guest) are incompatible. Basically, the 
function is returning -1 even if it is set to continue.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 src/cpu/cpu_ppc64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 2fedcd25da..23ea5a6a3e 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -517,7 +517,7 @@ virCPUppc64Compare(virCPUDefPtr host,
                    virCPUDefPtr cpu,
                    bool failIncompatible)
 {
-    virCPUCompareResult ret;
+    virCPUCompareResult ret = -1;
     g_autofree char *message = NULL;
 
     if (!host || !host->model) {
@@ -528,7 +528,7 @@ virCPUppc64Compare(virCPUDefPtr host,
             VIR_WARN("unknown host CPU");
             ret = VIR_CPU_COMPARE_INCOMPATIBLE;
         }
-        return -1;
+        return ret;
     }
 
     ret = ppc64Compute(host, cpu, NULL, &message);
-- 
2.25.1

Re: [PATCH] cpu_ppc64: compare CPU function is ignoring return value
Posted by Ján Tomko 3 years, 6 months ago
On a Tuesday in 2020, Julio Faracco wrote:
>Function to compare CPU on 64-bit PowerPC is ignoring the flag to avoid
>failure in case of CPUs (host and guest) are incompatible. Basically, the
>function is returning -1 even if it is set to continue.
>
>Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
>---
> src/cpu/cpu_ppc64.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
>index 2fedcd25da..23ea5a6a3e 100644
>--- a/src/cpu/cpu_ppc64.c
>+++ b/src/cpu/cpu_ppc64.c
>@@ -517,7 +517,7 @@ virCPUppc64Compare(virCPUDefPtr host,
>                    virCPUDefPtr cpu,
>                    bool failIncompatible)
> {
>-    virCPUCompareResult ret;
>+    virCPUCompareResult ret = -1;
>     g_autofree char *message = NULL;
>
>     if (!host || !host->model) {
>@@ -528,7 +528,7 @@ virCPUppc64Compare(virCPUDefPtr host,
>             VIR_WARN("unknown host CPU");
>             ret = VIR_CPU_COMPARE_INCOMPATIBLE;

This would be easier to read with each of the if/else
branches having their own return with the constant.

No need to assing to ret just to return it in the next
statement.

>         }
>-        return -1;

VIR_CPU_COMPARE_ERROR has the value of -1

Jano

>+        return ret;
>     }
>
>     ret = ppc64Compute(host, cpu, NULL, &message);
>-- 
>2.25.1
>
Re: [PATCH] cpu_ppc64: compare CPU function is ignoring return value
Posted by Julio Faracco 3 years, 6 months ago
Hi Jano,

Your suggestion makes more sense.
Let me send a V2 then.

Tks :-)

--
Julio Cesar Faracco

Em ter., 27 de out. de 2020 às 12:24, Ján Tomko <jtomko@redhat.com> escreveu:
>
> On a Tuesday in 2020, Julio Faracco wrote:
> >Function to compare CPU on 64-bit PowerPC is ignoring the flag to avoid
> >failure in case of CPUs (host and guest) are incompatible. Basically, the
> >function is returning -1 even if it is set to continue.
> >
> >Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> >---
> > src/cpu/cpu_ppc64.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
> >index 2fedcd25da..23ea5a6a3e 100644
> >--- a/src/cpu/cpu_ppc64.c
> >+++ b/src/cpu/cpu_ppc64.c
> >@@ -517,7 +517,7 @@ virCPUppc64Compare(virCPUDefPtr host,
> >                    virCPUDefPtr cpu,
> >                    bool failIncompatible)
> > {
> >-    virCPUCompareResult ret;
> >+    virCPUCompareResult ret = -1;
> >     g_autofree char *message = NULL;
> >
> >     if (!host || !host->model) {
> >@@ -528,7 +528,7 @@ virCPUppc64Compare(virCPUDefPtr host,
> >             VIR_WARN("unknown host CPU");
> >             ret = VIR_CPU_COMPARE_INCOMPATIBLE;
>
> This would be easier to read with each of the if/else
> branches having their own return with the constant.
>
> No need to assing to ret just to return it in the next
> statement.
>
> >         }
> >-        return -1;
>
> VIR_CPU_COMPARE_ERROR has the value of -1
>
> Jano
>
> >+        return ret;
> >     }
> >
> >     ret = ppc64Compute(host, cpu, NULL, &message);
> >--
> >2.25.1
> >