Re: [PATCH] erofs: [PATCH v2] Prevent entering an infinite loop when i is 0

[Gao Xiang]

Hi liujinbao1,

Since it's a trivial bug which can be simply fixed, I just
would like to confirm if you still have time and interest to
send a workable patch (after a week).

If you don't have time to follow up, I will ask Hongzhen to
fix this with your "Reported-by:" tomorrow.

Thanks,
Gao Xiang

On 2024/8/23 20:03, Gao Xiang wrote:
> On 2024/8/23 11:05, liujinbao1 wrote:
> > From: liujinbao1 <liujinbao1@xxxxxxxxxx>
> >
> > When i=0 and err is not equal to 0,
> > the while(-1) loop will enter into an
> > infinite loop. This patch avoids this issue.
> >
> > Signed-off-by: liujinbao1 <liujinbao1@xxxxxxxxxx>
> > ---
> >   fs/erofs/decompressor.c | 8 +++-----
> >   1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > > Hi,
>
> The patch is corrupted and the patch subject
> line is also broken.
>
> I think it should be:
> [PATCH v2] erofs: prevent entering an infinite loop when i is 0
>
> > > On 2024/8/22 14:27, liujinbao1 wrote:
> > > > From: liujinbao1 <liujinbao1@xxxxxxxxxx>
> > > >
> > > > When i=0 and err is not equal to 0,
> > > > the while(-1) loop will enter into an
> > > > infinite loop. This patch avoids this issue.
> > >
> > > Missing your Signed-off-by here.
> > >
> > > > ---
> > > >   fs/erofs/decompressor.c | 2 ++
> > > >   1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c index
> > > > c2253b6a5416..1b2b8cc7911c 100644
> > > > --- a/fs/erofs/decompressor.c
> > > > +++ b/fs/erofs/decompressor.c
> > > > @@ -539,6 +539,8 @@ int __init z_erofs_init_decompressor(void)
> > > >       for (i = 0; i < Z_EROFS_COMPRESSION_MAX; ++i) {
> > > >               err = z_erofs_decomp[i] ? z_erofs_decomp[i]->init() : 0;
> > > >               if (err) {
> > > > +                    if (!i)
> > > > +                            return err;
> > > >                       while (--i)
> > > >                               if (z_erofs_decomp[i])
> > > >                                       z_erofs_decomp[i]->exit();
> > >
> > >
> > > Thanks for catching this, how about the following diff (space-demaged).
> > >
> > > If it looks good to you, could you please send another version?
> >
> > > diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c index c2253b6a5416..c9b2bc1309d2 100644
> > > --- a/fs/erofs/decompressor.c
> > > +++ b/fs/erofs/decompressor.c
> > > @@ -534,18 +534,16 @@ int z_erofs_parse_cfgs(struct super_block *sb, struct erofs_super_block *dsb)
> > >
> > > int __init z_erofs_init_decompressor(void)
> > > {
> > > -      int i, err;
> > > +      int i, err = 0;
> > >
> > >         for (i = 0; i < Z_EROFS_COMPRESSION_MAX; ++i) {
> > >                 err = z_erofs_decomp[i] ? z_erofs_decomp[i]->init() : 0;
> > > -              if (err) {
> > > +              if (err && i)
> > >                         while (--i)
> > >                                 if (z_erofs_decomp[i])
> > >                                         z_erofs_decomp[i]->exit();
> > > -                      return err;
> > +                        break;
> > > -              }
> > >         }
> > > -      return 0;
> > > +      return err;
> > > }
> > missing break?
>
> Why needing a break?
>
> Thanks,
> Gao Xiang