Re: [PATCH v2 1/3] lib: zstd: Fix unused variable warning

[Geert Uytterhoeven]

Hi Nick,

On Wed, Nov 17, 2021 at 9:08 PM Nick Terrell <nickrterrell@xxxxxxxxx> wrote:
> The variable `litLengthSum` is only used by an `assert()`, so when
> asserts are disabled the compiler doesn't see any usage and warns.
>
> This issue is already fixed upstream by PR #2838 [0]. It was reported
> by the Kernel test robot in [1].
>
> Another approach would be to change zstd's disabled `assert()`
> definition to use the argument in a disabled branch, instead of
> ignoring the argument. I've avoided this approach because there are
> some small changes necessary to get zstd to build, and I would
> want to thoroughly re-test for performance, since that is slightly
> changing the code in every function in zstd. It seems like a
> trivial change, but some functions are pretty sensitive to small
> changes. However, I think it is a valid approach that I would
> like to see upstream take, so I've opened Issue #2868 to attempt
> this upstream.
>
> [0] https://github.com/facebook/zstd/pull/2838
> [1] https://lore.kernel.org/linux-mm/202111120312.833wII4i-lkp@xxxxxxxxx/T/
> [2] https://github.com/facebook/zstd/issues/2868
>
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Signed-off-by: Nick Terrell <terrelln@xxxxxx>

Thanks for your patch!

> --- a/lib/zstd/compress/zstd_compress_superblock.c
> +++ b/lib/zstd/compress/zstd_compress_superblock.c
> @@ -411,6 +411,8 @@ static size_t ZSTD_seqDecompressedSize(seqStore_t const* seqStore, const seqDef*
>      const seqDef* sp = sstart;
>      size_t matchLengthSum = 0;
>      size_t litLengthSum = 0;
> +    /* Only used by assert(), suppress unused variable warnings in production. */
> +    (void)litLengthSum;

The Linux way-to do this is to add __maybe_unused.
But perhaps you don't want to introduce that in the upstream codebase.

>      while (send-sp > 0) {
>          ZSTD_sequenceLength const seqLen = ZSTD_getSequenceLength(seqStore, sp);
>          litLengthSum += seqLen.litLength;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds