Re: [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h
From: haoqin huang
Date: Wed Jul 29 2026 - 22:52:07 EST
On Wed, Jul 29, 2026 at 12:50 PM Sergey Senozhatsky
<senozhatsky@xxxxxxxxxxxx> wrote:
>
> On (26/07/29 12:32), haoqin huang wrote:
> > > > > > -#define ZSTD_MAX_CLEVEL 22
> > > > > > -
> > > > > > __attribute__((__unused__))
> > > > >
> > > > > Sashiko made a good point. Can we use zstd_max_clevel() instead?
> > > >
> > > > Good point, I'll drop this patch and use zstd_max_clevel() instead
> > > > in v3.
> > > >
> > > > Since it's a runtime function and can't be used for static struct
> > > > initialization, I plan to set backend_zstd's level_max to -1 as a
> > > > sentinel value, and query the actual maximum in
> > > > zcomp_validate_params():
> > > >
> > > > s32 max = backend->level_max;
> > > > if (max < 0)
> > > > max = zstd_max_clevel();
> > > >
> > > > Do you think this approach is feasible?
> > >
> > > Hmm, no, that doesn't look good. zcomp should not include
> > > zstd.h or any other libs directly. Should params validation
> > > be a per-backend callback then?
> >
> > Good point, zcomp.c shouldn't include library headers. A per-backend
> > callback would be cleaner.
> >
> > I'll add an optional validate_params to zcomp_ops: the zstd backend
> > implements it using zstd_max_clevel() internally, while lzo/deflate
> > and others just rely on the static caps check (no callback needed).
> > zcomp.c stays free of any library headers.
>
> I'm actually leaning towards validation in .setup_params() now.
> It's not immediate but, first, it doesn't matter that much, we still
> don't create zram with invalid params configuration and, second, it
> sort of makes sense to do validation in .setup_params(). We already
> started doing that for deflate winbits (a patch from earlier today).
>
> Can you please add your validation to per-backend .setup_params()?
Agreed, I'll move the validation to zstd_setup_params() in v3.