| Message ID | a5f00432063ead8d4ae09315c1b09617a12b22f7.1719274203.git.victorm.lira@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN] common/sched: address a violation of MISRA C Rule 17.7 | expand |
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
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;