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

[haoqin huang]

On Tue, Aug 4, 2026 at 1:37 PM Sergey Senozhatsky
<senozhatsky@xxxxxxxxxxxx> 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 == ZCOMP_PARAM_NOT_SET)
> >               params->level = 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 consistency.

> > +     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->level);
> > +             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?