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

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (26/08/04 15:13), haoqin huang wrote:
> > On (26/08/03 22:12), Haoqin Huang wrote:
> > [..]
> > >  static int setup_params_842(struct zcomp_params *params)
> > >  {
> > > +     if (params->dict_sz) {
> > > +             pr_err("842: dictionary is not supported\n");
> > > +             return -EOPNOTSUPP;
> > > +     }
> > > +     if (params->level != ZCOMP_PARAM_NOT_SET) {
> > > +             pr_err("842: compression level is not supported\n");
> > > +             return -EOPNOTSUPP;
> > > +     }
> > >       return 0;
> > >  }
> > [..]
> > >  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;
> > > +     else if (params->level < Z_DEFAULT_COMPRESSION ||
> > > +              params->level > Z_BEST_COMPRESSION) {
> > > +             pr_err("deflate: invalid compression level %d\n", params->level);
> > > +             return -EINVAL;
> > > +     }
> >
> > This is purely optional, if you add per-backend pr_fmt() with backend
> > name e.g. "zstd:","lz4:" and so on (in a separate patch) then you don't
> > need to explicitly prefix every pr_err().
> 
> I considered adding pr_fmt, but the existing winbits message already
> has "deflate" inline:
> 
>     pr_err("invalid deflate winbits: %d\n", wb);

Feel free to remove it, if you add per-backend pr_fmt().

> which would become "deflate: invalid deflate winbits:" and look redundant.
> I can add pr_fmt in a separate patch and clean that up to just
> "invalid winbits %d" at the same time though, if you prefer.

Sounds good.