Hi Andy, On 2018/5/22 6:13, Andy Shevchenko wrote: > On Mon, May 21, 2018 at 2:58 PM, Yisheng Xie <xieyishe...@huawei.com> wrote: >> match_string() returns the index of an array for a matching string, >> which can be used intead of open coded variant. >> >> Cc: Ingo Molnar <mi...@redhat.com> >> Cc: Peter Zijlstra <pet...@infradead.org> >> Signed-off-by: Yisheng Xie <xieyishe...@huawei.com> >> --- >> kernel/sched/debug.c | 20 +++++++++----------- >> 1 file changed, 9 insertions(+), 11 deletions(-) >> >> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c >> index 15b10e2..9e34499 100644 >> --- a/kernel/sched/debug.c >> +++ b/kernel/sched/debug.c >> @@ -111,16 +111,14 @@ static int sched_feat_set(char *cmp) >> cmp += 3; >> } >> >> - for (i = 0; i < __SCHED_FEAT_NR; i++) { >> - if (strcmp(cmp, sched_feat_names[i]) == 0) { >> - if (neg) { >> - sysctl_sched_features &= ~(1UL << i); >> - sched_feat_disable(i); >> - } else { >> - sysctl_sched_features |= (1UL << i); >> - sched_feat_enable(i); >> - } >> - break; >> + i = match_string(sched_feat_names, __SCHED_FEAT_NR, cmp); > >> + if (i >= 0) { > > Why not > > if (i < 0) > return i;
if i >=0 it will also return i. so need return i just if (i < 0), right ? > > ? > >> + if (neg) { >> + sysctl_sched_features &= ~(1UL << i); >> + sched_feat_disable(i); >> + } else { >> + sysctl_sched_features |= (1UL << i); >> + sched_feat_enable(i); >> } >> } >> >> @@ -150,7 +148,7 @@ static int sched_feat_set(char *cmp) >> inode_lock(inode); >> i = sched_feat_set(cmp); >> inode_unlock(inode); > >> - if (i == __SCHED_FEAT_NR) >> + if (i < 0) >> return -EINVAL; > > Now it would be > > if (i < 0) > return i; Right, will change it in next version Thanks Yisheng Xie >