From: Victor Lira <victorm.lira@amd.com>
Rule 17.7: "The value returned by a function having non-void return type
shall be used"
This patch fixes this by adding a check to the return value.
No functional changes.
Signed-off-by: Victor Lira <victorm.lira@amd.com>
---
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Dario Faggioli <dfaggioli@suse.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: xen-devel@lists.xenproject.org
---
xen/common/sched/core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index d84b65f197..e1cd824622 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -2789,7 +2789,10 @@ static int cpu_schedule_up(unsigned int cpu)
BUG_ON(cpu >= NR_CPUS);
if ( idle_vcpu[cpu] == NULL )
- vcpu_create(idle_vcpu[0]->domain, cpu);
+ {
+ if ( vcpu_create(idle_vcpu[0]->domain, cpu) == NULL )
+ return -ENOMEM;
+ }
else
idle_vcpu[cpu]->sched_unit->res = sr;
--
2.37.6
On 25.06.2024 02:15, victorm.lira@amd.com wrote: > From: Victor Lira <victorm.lira@amd.com> > > Rule 17.7: "The value returned by a function having non-void return type > shall be used" > > This patch fixes this by adding a check to the return value. > No functional changes. > > Signed-off-by: Victor Lira <victorm.lira@amd.com> > --- > Cc: George Dunlap <george.dunlap@citrix.com> > Cc: Dario Faggioli <dfaggioli@suse.com> > Cc: Juergen Gross <jgross@suse.com> > Cc: xen-devel@lists.xenproject.org > --- > xen/common/sched/core.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c > index d84b65f197..e1cd824622 100644 > --- a/xen/common/sched/core.c > +++ b/xen/common/sched/core.c > @@ -2789,7 +2789,10 @@ static int cpu_schedule_up(unsigned int cpu) > BUG_ON(cpu >= NR_CPUS); > > if ( idle_vcpu[cpu] == NULL ) > - vcpu_create(idle_vcpu[0]->domain, cpu); > + { > + if ( vcpu_create(idle_vcpu[0]->domain, cpu) == NULL ) > + return -ENOMEM; > + } First: Two such if()s want folding. > else > idle_vcpu[cpu]->sched_unit->res = sr; > Then: Down from here there is if ( idle_vcpu[cpu] == NULL ) return -ENOMEM; which your change is rendering redundant for at least the vcpu_create() path. Finally, as we're touching error handling here (and mayby more a question to the maintainers than to you): What about sr in the error case? It's being allocated earlier in the function, but not freed upon error. Hmm, looks like cpu_schedule_down() is assumed to be taking care of the case, yet then I wonder how that can assume that get_sched_res() would return non-NULL - afaict it may be called without cpu_schedule_up() having run first, or with it having bailed early with -ENOMEM. Jan
On 25.06.24 08:34, Jan Beulich wrote: > On 25.06.2024 02:15, victorm.lira@amd.com wrote: >> From: Victor Lira <victorm.lira@amd.com> >> >> Rule 17.7: "The value returned by a function having non-void return type >> shall be used" >> >> This patch fixes this by adding a check to the return value. >> No functional changes. >> >> Signed-off-by: Victor Lira <victorm.lira@amd.com> >> --- >> Cc: George Dunlap <george.dunlap@citrix.com> >> Cc: Dario Faggioli <dfaggioli@suse.com> >> Cc: Juergen Gross <jgross@suse.com> >> Cc: xen-devel@lists.xenproject.org >> --- >> xen/common/sched/core.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c >> index d84b65f197..e1cd824622 100644 >> --- a/xen/common/sched/core.c >> +++ b/xen/common/sched/core.c >> @@ -2789,7 +2789,10 @@ static int cpu_schedule_up(unsigned int cpu) >> BUG_ON(cpu >= NR_CPUS); >> >> if ( idle_vcpu[cpu] == NULL ) >> - vcpu_create(idle_vcpu[0]->domain, cpu); >> + { >> + if ( vcpu_create(idle_vcpu[0]->domain, cpu) == NULL ) >> + return -ENOMEM; >> + } > > First: Two such if()s want folding. > >> else >> idle_vcpu[cpu]->sched_unit->res = sr; >> > > Then: Down from here there is > > if ( idle_vcpu[cpu] == NULL ) > return -ENOMEM; > > which your change is rendering redundant for at least the vcpu_create() > path. > > Finally, as we're touching error handling here (and mayby more a question > to the maintainers than to you): What about sr in the error case? It's > being allocated earlier in the function, but not freed upon error. Hmm, > looks like cpu_schedule_down() is assumed to be taking care of the case, > yet then I wonder how that can assume that get_sched_res() would return > non-NULL - afaict it may be called without cpu_schedule_up() having run > first, or with it having bailed early with -ENOMEM. Yes, you are right. cpu_schedule_down() should bail out early in case sr is NULL. I'll write a patch. Juergen
© 2016 - 2024 Red Hat, Inc.