Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210327120155.500-1-ionic@ionic.de> <20210328095227.24323-1-ionic@ionic.de>
 <CAK7LNATzx9ujmapPbPzjL1Yo-A0RAXz0Kma+ve8pUeDDVx8GGw@mail.gmail.com> <9e8d429f-c21c-7d9e-0dcd-8947846fe9ba@ionic.de>
In-Reply-To: <9e8d429f-c21c-7d9e-0dcd-8947846fe9ba@ionic.de>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Sat, 10 Apr 2021 18:12:54 +0900
Message-ID: <CAK7LNARzoCoeTtTxyver_XsLd_hCC3Tbs++GU_bAx7q1Crc20Q@mail.gmail.com>
Subject: Re: [PATCH v2] kconfig: nconf: stop endless search-up loops
To:     Mihai Moldovan <ionic@ionic.de>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Apr 10, 2021 at 4:00 PM Mihai Moldovan <ionic@ionic.de> wrote:
>
> * On 4/10/21 7:47 AM, Masahiro Yamada wrote:
> > On Sun, Mar 28, 2021 at 6:52 PM Mihai Moldovan <ionic@ionic.de> wrote:
> >> +               if ((index == -1) && (index == match_start))
> >> +                       return -1;
> >
> > We know 'index' is -1 in the second comparison.
> > So, you can also write like this:
> >
> >        if (match_start == -1 && index == -1)
> >                 return -1;
>
> I know, but I sided for the other form for semantic reasons - this more closely
> directly describes what we actually care about (both being the same value and
> either one being -1).
>
>
> > But, it is not the correct fix, either.
> >
> > The root cause of the bug is match_start
> > becoming -1.
> >
> >
> > The following is the correct way to fix the bug
> > without increasing the number of lines.
> >
> >
> >
> > diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
> > index e0f965529166..af814b39b876 100644
> > [...]
> > +       match_start = (match_start + items_num) % items_num;
> >         index = match_start;
> > -       index = (index + items_num) % items_num;
>
> This is probably more elegant and fixes two issues at the same time: match_start
> becoming -1 or n (which is likewise invalid, but was implicitly handled through
> the remainder operation).
>
> No objections from my side.


Could you send v3 please?

Then, I will apply it.


-- 
Best Regards
Masahiro Yamada