Re: [PATCH v5 3/4] zram: validate parameters in each backend's setup_params

haoqin huang <[email protected]> Tue, 4 Aug 2026 15:02:15 +0800
Newsgroups org.kernel.vger.linux-block,org.kernel.vger.linux-kernel
Message-ID <CAEjiKSnECdUd9oZNbhJLiD5NTpOY+fx=mKNnccCXtBLyEC7KCQ@mail.gmail.com>
On Tue, Aug 4, 2026 at 1:37=E2=80=AFPM Sergey Senozhatsky
<[email protected]> wrote:
>
> On (26/08/03 22:12), Haoqin Huang wrote:
> >  static int deflate_setup_params(struct zcomp_params *params)
> >  {
> > +     if (params->dict_sz) {
> > +             pr_err("deflate: dictionary is not supported\n");
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> >       if (params->level =3D=3D ZCOMP_PARAM_NOT_SET)
> >               params->level =3D Z_DEFAULT_COMPRESSION;
>
> If we want to be pedantic, then {} should also be added to the "if"
> in this case.  And in other similar cases.
>

Okay, thanks for the reminder. I will add {} to the if-branch for consisten=
cy.

> > +     else if (params->level < Z_DEFAULT_COMPRESSION ||
> > +              params->level > Z_BEST_COMPRESSION) {
> > +             pr_err("deflate: invalid compression level %d\n", params-=
>level);
> > +             return -EINVAL;
> > +     }
> > +
>
> [..]
> > +     else if (params->level < 1 || params->level > LZ4HC_MAX_CLEVEL) {
> > +             pr_err("lz4hc: invalid compression level %d\n", params->l=
evel);
> > +             return -EINVAL;
> > +     }
>
> Let's add a small comment justifying/explaining that "1" constant?

For the lz4hc comment, how about:
        /*
         * LZ4HC_compress_generic() clamps levels below 1 to
         * LZ4HC_DEFAULT_CLEVEL, so < 1 is the real lower bound.
         */
Does that look reasonable?