Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Thu, 18 Nov 2021 08:50:20 +0100 | Subject | Re: [PATCH v2 1/3] lib: zstd: Fix unused variable warning |
| |
Hi Nick,
On Wed, Nov 17, 2021 at 9:08 PM Nick Terrell <nickrterrell@gmail.com> 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@intel.com/T/ > [2] https://github.com/facebook/zstd/issues/2868 > > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Nick Terrell <terrelln@fb.com>
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@linux-m68k.org
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
| |