Project

General

Profile

Bug #11667

remove duplicate lz4 implementations

Added by Toomas Soome about 1 month ago. Updated 18 days ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
zfs - Zettabyte File System
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

Use one lz4 implementation. That is, move uts/common/fs/zfs/lz4.c to common/lz4/, patch it up to be buildable from kernel, loader and userland and remove unused static bits + comments.

Build cleanups are based on gcc (7,9) and clang80 notes (oi and freebsd) - mostly concerning about alignment changes, const vs non-const.

Testing done: build/install/boot. I have compression=on so the pool access is immediately tested by loader and kernel zfs; the built in loader font and softcore dictionary is decompressed, so both build time compress and boot time decompress are also tested by boot.

History

#1

Updated by Toomas Soome about 1 month ago

  • Description updated (diff)
#2

Updated by Toomas Soome about 1 month ago

Logging the updates to have lz4 build based on lz4.c in zfs tree. First of all, we obviously need to add proper headers to support loader/userland build space. After that we can spot first set of errors:

/code/illumos-gate/usr/src/common/lz4/lz4.c: In function 'lz4_compress':
/code/illumos-gate/usr/src/common/lz4/lz4.c:93:22: warning: implicit declaration of function 'BE_32' [-Wimplicit-function-declaration]
  *(uint32_t *)dest = BE_32(bufsiz);
                      ^~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c: In function 'lz4_decompress':
/code/illumos-gate/usr/src/common/lz4/lz4.c:103:20: warning: implicit declaration of function 'BE_IN32' [-Wimplicit-function-declaration]
  uint32_t bufsiz = BE_IN32(src);
                    ^~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c: In function 'real_LZ4_compress':
/code/illumos-gate/usr/src/common/lz4/lz4.c:924:14: warning: implicit declaration of function 'kmem_zalloc'; did you mean 'Realloc'? [-Wimplicit-function-declaration]
  void *ctx = kmem_zalloc(sizeof (struct refTables), KM_NOSLEEP);
              ^~~~~~~~~~~
              Realloc
/code/illumos-gate/usr/src/common/lz4/lz4.c:924:53: error: 'KM_NOSLEEP' undeclared (first use in this function)
  void *ctx = kmem_zalloc(sizeof (struct refTables), KM_NOSLEEP);
                                                     ^~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:924:53: note: each undeclared identifier is reported only once for each function it appears in
/code/illumos-gate/usr/src/common/lz4/lz4.c:939:2: warning: implicit declaration of function 'kmem_free' [-Wimplicit-function-declaration]
  kmem_free(ctx, sizeof (struct refTables));
  ^~~~~~~~~
At top level:
/code/illumos-gate/usr/src/common/lz4/lz4.c:961:1: warning: 'real_LZ4_uncompress' defined but not used [-Wunused-function]
 real_LZ4_uncompress(const char *source, char *dest, int osize)
 ^~~~~~~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:536:1: warning: 'LZ4_compressBound' defined but not used [-Wunused-function]
 LZ4_compressBound(int isize)
 ^~~~~~~~~~~~~~~~~

To address the issues above, we need to have alternatives for endian macros, memory allocation and we need to remove unused code.

Adding -Wextra does bring up additional warnings:

/code/illumos-gate/usr/src/common/lz4/lz4.c: In function 'lz4_compress':
/code/illumos-gate/usr/src/common/lz4/lz4.c:71:76: warning: unused parameter 'n' [-Wunused-parameter]
   71 | lz4_compress(void *s_start, void *d_start, size_t s_len, size_t d_len, int n)
      |                                                                        ~~~~^
/code/illumos-gate/usr/src/common/lz4/lz4.c: In function 'lz4_decompress':
/code/illumos-gate/usr/src/common/lz4/lz4.c:102:78: warning: unused parameter '' [-Wunused-parameter]
  102 | lz4_decompress(void *s_start, void *d_start, size_t s_len, size_t d_len, int n)
      |                                                                          ~~~~^

So we need to mark unused parameters with __unused.

Adding -Wcast-qual does bring up more issues about mishandling const qualifier:

/code/illumos-gate/usr/src/common/lz4/lz4.c: In function 'LZ4_compressCtx':
/code/illumos-gate/usr/src/common/lz4/lz4.c:564:19: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  564 |  const BYTE *ip = (BYTE *) source;
      |                   ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:442:33: note: in definition of macro 'LZ4_HASH_FUNCTION'
  442 | #define LZ4_HASH_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH * 8) - \
      |                                 ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:444:45: note: in expansion of macro 'A32'
  444 | #define LZ4_HASH_VALUE(p) LZ4_HASH_FUNCTION(A32(p))
      |                                             ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:584:12: note: in expansion of macro 'LZ4_HASH_VALUE'
  584 |  HashTable[LZ4_HASH_VALUE(ip)] = ip - base;
      |            ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:442:33: note: in definition of macro 'LZ4_HASH_FUNCTION'
  442 | #define LZ4_HASH_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH * 8) - \
      |                                 ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:444:45: note: in expansion of macro 'A32'
  444 | #define LZ4_HASH_VALUE(p) LZ4_HASH_FUNCTION(A32(p))
      |                                             ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:586:13: note: in expansion of macro 'LZ4_HASH_VALUE'
  586 |  forwardH = LZ4_HASH_VALUE(ip);
      |             ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:442:33: note: in definition of macro 'LZ4_HASH_FUNCTION'
  442 | #define LZ4_HASH_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH * 8) - \
      |                                 ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:444:45: note: in expansion of macro 'A32'
  444 | #define LZ4_HASH_VALUE(p) LZ4_HASH_FUNCTION(A32(p))
      |                                             ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:606:15: note: in expansion of macro 'LZ4_HASH_VALUE'
  606 |    forwardH = LZ4_HASH_VALUE(forwardIp);
      |               ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:610:42: note: in expansion of macro 'A32'
  610 |   } while ((ref < ip - MAX_DISTANCE) || (A32(ref) != A32(ip)));
      |                                          ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:610:54: note: in expansion of macro 'A32'
  610 |   } while ((ref < ip - MAX_DISTANCE) || (A32(ref) != A32(ip)));
      |                                                      ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:613:34: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  613 |   while ((ip > anchor) && (ref > (BYTE *) source) &&
      |                                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:417:37: note: in expansion of macro 'A32'
  417 | #define LZ4_COPYSTEP(s, d) A32(d) = A32(s); d += 4; s += 4;
      |                                     ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:418:30: note: in expansion of macro 'LZ4_COPYSTEP'
  418 | #define LZ4_COPYPACKET(s, d) LZ4_COPYSTEP(s, d); LZ4_COPYSTEP(s, d);
      |                              ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:445:36: note: in expansion of macro 'LZ4_COPYPACKET'
  445 | #define LZ4_WILDCOPY(s, d, e) do { LZ4_COPYPACKET(s, d) } while (d < e);
      |                                    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:446:53: note: in expansion of macro 'LZ4_WILDCOPY'
  446 | #define LZ4_BLINDCOPY(s, d, l) { BYTE* e = (d) + l; LZ4_WILDCOPY(s, d, e); \
      |                                                     ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:638:3: note: in expansion of macro LZ4_BLINDCOPY'
  638 |   LZ4_BLINDCOPY(anchor, op, length);
      |   ^~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:417:37: note: in expansion of macro 'A32'
  417 | #define LZ4_COPYSTEP(s, d) A32(d) = A32(s); d += 4; s += 4;
      |                                     ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:418:50: note: in expansion of macro 'LZ4_COPYSTEP'
  418 | #define LZ4_COPYPACKET(s, d) LZ4_COPYSTEP(s, d); LZ4_COPYSTEP(s, d);
      |                                                  ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:445:36: note: in expansion of macro 'LZ4_COPYPACKET'
  445 | #define LZ4_WILDCOPY(s, d, e) do { LZ4_COPYPACKET(s, d) } while (d < e);
      |                                    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:446:53: note: in expansion of macro 'LZ4_WILDCOPY'
  446 | #define LZ4_BLINDCOPY(s, d, l) { BYTE* e = (d) + l; LZ4_WILDCOPY(s, d, e); \
      |                                                     ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:638:3: note: in expansion of macro LZ4_BLINDCOPY'
  638 |   LZ4_BLINDCOPY(anchor, op, length);
      |   ^~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:416:15: note: in expansion of macro 'A32'
  416 | #define AARCH A32
      |               ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:649:17: note: in expansion of macro 'AARCH'
  649 |    UARCH diff = AARCH(ref) ^ AARCH(ip);
      |                 ^~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:416:15: note: in expansion of macro 'A32'
  416 | #define AARCH A32
      |               ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:649:30: note: in expansion of macro 'AARCH'
  649 |    UARCH diff = AARCH(ref) ^ AARCH(ip);
      |                              ^~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:368:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  368 | #define A16(x) (((U16_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:664:35: note: in expansion of macro 'A16'
  664 |   if ((ip < (matchlimit - 1)) && (A16(ref) == A16(ip))) {
      |                                   ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:368:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  368 | #define A16(x) (((U16_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:664:47: note: in expansion of macro 'A16'
  664 |   if ((ip < (matchlimit - 1)) && (A16(ref) == A16(ip))) {
      |                                               ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:442:33: note: in definition of macro 'LZ4_HASH_FUNCTION'
  442 | #define LZ4_HASH_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH * 8) - \
      |                                 ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:444:45: note: in expansion of macro 'A32'
  444 | #define LZ4_HASH_VALUE(p) LZ4_HASH_FUNCTION(A32(p))
      |                                             ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:698:13: note: in expansion of macro 'LZ4_HASH_VALUE'
  698 |   HashTable[LZ4_HASH_VALUE(ip - 2)] = ip - 2 - base;
      |             ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:442:33: note: in definition of macro 'LZ4_HASH_FUNCTION'
  442 | #define LZ4_HASH_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH * 8) - \
      |                                 ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:444:45: note: in expansion of macro 'A32'
  444 | #define LZ4_HASH_VALUE(p) LZ4_HASH_FUNCTION(A32(p))
      |                                             ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:701:26: note: in expansion of macro 'LZ4_HASH_VALUE'
  701 |   ref = base + HashTable[LZ4_HASH_VALUE(ip)];
      |                          ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:442:33: note: in definition of macro 'LZ4_HASH_FUNCTION'
  442 | #define LZ4_HASH_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH * 8) - \
      |                                 ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:444:45: note: in expansion of macro 'A32'
  444 | #define LZ4_HASH_VALUE(p) LZ4_HASH_FUNCTION(A32(p))
      |                                             ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:702:13: note: in expansion of macro 'LZ4_HASH_VALUE'
  702 |   HashTable[LZ4_HASH_VALUE(ip)] = ip - base;
      |             ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:703:43: note: in expansion of macro 'A32'
  703 |   if ((ref > ip - (MAX_DISTANCE + 1)) && (A32(ref) == A32(ip))) {
      |                                           ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:703:55: note: in expansion of macro 'A32'
  703 |   if ((ref > ip - (MAX_DISTANCE + 1)) && (A32(ref) == A32(ip))) {
      |                                                       ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:442:33: note: in definition of macro 'LZ4_HASH_FUNCTION'
  442 | #define LZ4_HASH_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH * 8) - \
      |                                 ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:444:45: note: in expansion of macro 'A32'
  444 | #define LZ4_HASH_VALUE(p) LZ4_HASH_FUNCTION(A32(p))
      |                                             ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:710:14: note: in expansion of macro 'LZ4_HASH_VALUE'
  710 |   forwardH = LZ4_HASH_VALUE(ip);
      |              ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c: In function 'LZ4_compress64kCtx':
/code/illumos-gate/usr/src/common/lz4/lz4.c:759:19: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  759 |  const BYTE *ip = (BYTE *) source;
      |                   ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:743:36: note: in definition of macro 'LZ4_HASH64K_FUNCTION'
  743 | #define LZ4_HASH64K_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH*8) - \
      |                                    ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:745:51: note: in expansion of macro 'A32'
  745 | #define LZ4_HASH64K_VALUE(p) LZ4_HASH64K_FUNCTION(A32(p))
      |                                                   ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:779:13: note: in expansion of macro 'LZ4_HASH64K_VALUE'
  779 |  forwardH = LZ4_HASH64K_VALUE(ip);
      |             ^~~~~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:743:36: note: in definition of macro 'LZ4_HASH64K_FUNCTION'
  743 | #define LZ4_HASH64K_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH*8) - \
      |                                    ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:745:51: note: in expansion of macro 'A32'
  745 | #define LZ4_HASH64K_VALUE(p) LZ4_HASH64K_FUNCTION(A32(p))
      |                                                   ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:799:15: note: in expansion of macro 'LZ4_HASH64K_VALUE'
  799 |    forwardH = LZ4_HASH64K_VALUE(forwardIp);
      |               ^~~~~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:803:12: note: in expansion of macro 'A32'
  803 |   } while (A32(ref) != A32(ip));
      |            ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:803:24: note: in expansion of macro 'A32'
  803 |   } while (A32(ref) != A32(ip));
      |                        ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:806:34: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  806 |   while ((ip > anchor) && (ref > (BYTE *) source) &&
      |                                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:417:37: note: in expansion of macro 'A32'
  417 | #define LZ4_COPYSTEP(s, d) A32(d) = A32(s); d += 4; s += 4;
      |                                     ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:418:30: note: in expansion of macro 'LZ4_COPYSTEP'
  418 | #define LZ4_COPYPACKET(s, d) LZ4_COPYSTEP(s, d); LZ4_COPYSTEP(s, d);
      |                              ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:445:36: note: in expansion of macro 'LZ4_COPYPACKET'
  445 | #define LZ4_WILDCOPY(s, d, e) do { LZ4_COPYPACKET(s, d) } while (d < e);
      |                                    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:446:53: note: in expansion of macro 'LZ4_WILDCOPY'
  446 | #define LZ4_BLINDCOPY(s, d, l) { BYTE* e = (d) + l; LZ4_WILDCOPY(s, d, e); \
      |                                                     ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:831:3: note: in expansion of macro LZ4_BLINDCOPY'
  831 |   LZ4_BLINDCOPY(anchor, op, length);
      |   ^~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:417:37: note: in expansion of macro 'A32'
  417 | #define LZ4_COPYSTEP(s, d) A32(d) = A32(s); d += 4; s += 4;
      |                                     ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:418:50: note: in expansion of macro 'LZ4_COPYSTEP'
  418 | #define LZ4_COPYPACKET(s, d) LZ4_COPYSTEP(s, d); LZ4_COPYSTEP(s, d);
      |                                                  ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:445:36: note: in expansion of macro 'LZ4_COPYPACKET'
  445 | #define LZ4_WILDCOPY(s, d, e) do { LZ4_COPYPACKET(s, d) } while (d < e);
      |                                    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:446:53: note: in expansion of macro 'LZ4_WILDCOPY'
  446 | #define LZ4_BLINDCOPY(s, d, l) { BYTE* e = (d) + l; LZ4_WILDCOPY(s, d, e); \
      |                                                     ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:831:3: note: in expansion of macro LZ4_BLINDCOPY'
  831 |   LZ4_BLINDCOPY(anchor, op, length);
      |   ^~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:416:15: note: in expansion of macro 'A32'
  416 | #define AARCH A32
      |               ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:842:17: note: in expansion of macro 'AARCH'
  842 |    UARCH diff = AARCH(ref) ^ AARCH(ip);
      |                 ^~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:416:15: note: in expansion of macro 'A32'
  416 | #define AARCH A32
      |               ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:842:30: note: in expansion of macro 'AARCH'
  842 |    UARCH diff = AARCH(ref) ^ AARCH(ip);
      |                              ^~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:368:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  368 | #define A16(x) (((U16_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:857:35: note: in expansion of macro 'A16'
  857 |   if ((ip < (matchlimit - 1)) && (A16(ref) == A16(ip))) {
      |                                   ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:368:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  368 | #define A16(x) (((U16_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:857:47: note: in expansion of macro 'A16'
  857 |   if ((ip < (matchlimit - 1)) && (A16(ref) == A16(ip))) {
      |                                               ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:743:36: note: in definition of macro 'LZ4_HASH64K_FUNCTION'
  743 | #define LZ4_HASH64K_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH*8) - \
      |                                    ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:745:51: note: in expansion of macro 'A32'
  745 | #define LZ4_HASH64K_VALUE(p) LZ4_HASH64K_FUNCTION(A32(p))
      |                                                   ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:891:13: note: in expansion of macro 'LZ4_HASH64K_VALUE'
  891 |   HashTable[LZ4_HASH64K_VALUE(ip - 2)] = ip - 2 - base;
      |             ^~~~~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:743:36: note: in definition of macro 'LZ4_HASH64K_FUNCTION'
  743 | #define LZ4_HASH64K_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH*8) - \
      |                                    ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:745:51: note: in expansion of macro 'A32'
  745 | #define LZ4_HASH64K_VALUE(p) LZ4_HASH64K_FUNCTION(A32(p))
      |                                                   ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:894:26: note: in expansion of macro 'LZ4_HASH64K_VALUE'
  894 |   ref = base + HashTable[LZ4_HASH64K_VALUE(ip)];
      |                          ^~~~~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:743:36: note: in definition of macro 'LZ4_HASH64K_FUNCTION'
  743 | #define LZ4_HASH64K_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH*8) - \
      |                                    ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:745:51: note: in expansion of macro 'A32'
  745 | #define LZ4_HASH64K_VALUE(p) LZ4_HASH64K_FUNCTION(A32(p))
      |                                                   ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:895:13: note: in expansion of macro 'LZ4_HASH64K_VALUE'
  895 |   HashTable[LZ4_HASH64K_VALUE(ip)] = ip - base;
      |             ^~~~~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:896:7: note: in expansion of macro A32'
  896 |   if (A32(ref) == A32(ip)) {
      |       ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:896:19: note: in expansion of macro 'A32'
  896 |   if (A32(ref) == A32(ip)) {
      |                   ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:743:36: note: in definition of macro 'LZ4_HASH64K_FUNCTION'
  743 | #define LZ4_HASH64K_FUNCTION(i) (((i) * 2654435761U) >> ((MINMATCH*8) - \
      |                                    ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:745:51: note: in expansion of macro 'A32'
  745 | #define LZ4_HASH64K_VALUE(p) LZ4_HASH64K_FUNCTION(A32(p))
      |                                                   ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:903:14: note: in expansion of macro 'LZ4_HASH64K_VALUE'
  903 |   forwardH = LZ4_HASH64K_VALUE(ip);
      |              ^~~~~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c: In function 'LZ4_uncompress_unknownOutputSize':
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:417:37: note: in expansion of macro 'A32'
  417 | #define LZ4_COPYSTEP(s, d) A32(d) = A32(s); d += 4; s += 4;
      |                                     ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:418:30: note: in expansion of macro 'LZ4_COPYSTEP'
  418 | #define LZ4_COPYPACKET(s, d) LZ4_COPYSTEP(s, d); LZ4_COPYSTEP(s, d);
      |                              ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:445:36: note: in expansion of macro 'LZ4_COPYPACKET'
  445 | #define LZ4_WILDCOPY(s, d, e) do { LZ4_COPYPACKET(s, d) } while (d < e);
      |                                    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:1030:3: note: in expansion of macro 'LZ4_WILDCOPY'
 1030 |   LZ4_WILDCOPY(ip, op, cpy);
      |   ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:417:37: note: in expansion of macro 'A32'
  417 | #define LZ4_COPYSTEP(s, d) A32(d) = A32(s); d += 4; s += 4;
      |                                     ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:418:50: note: in expansion of macro 'LZ4_COPYSTEP'
  418 | #define LZ4_COPYPACKET(s, d) LZ4_COPYSTEP(s, d); LZ4_COPYSTEP(s, d);
      |                                                  ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:445:36: note: in expansion of macro 'LZ4_COPYPACKET'
  445 | #define LZ4_WILDCOPY(s, d, e) do { LZ4_COPYPACKET(s, d) } while (d < e);
      |                                    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:1030:3: note: in expansion of macro 'LZ4_WILDCOPY'
 1030 |   LZ4_WILDCOPY(ip, op, cpy);
      |   ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:368:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  368 | #define A16(x) (((U16_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:430:55: note: in expansion of macro 'A16'
  430 | #define LZ4_READ_LITTLEENDIAN_16(d, s, p) { d = (s) - A16(p); }
      |                                                       ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:1035:3: note: in expansion of macro 'LZ4_READ_LITTLEENDIAN_16'
 1035 |   LZ4_READ_LITTLEENDIAN_16(ref, cpy, ip);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:1068:14: note: in expansion of macro 'A32'
 1068 |    A32(op) = A32(ref);
      |              ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:417:37: note: in expansion of macro 'A32'
  417 | #define LZ4_COPYSTEP(s, d) A32(d) = A32(s); d += 4; s += 4;
      |                                     ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:1072:4: note: in expansion of macro 'LZ4_COPYSTEP'
 1072 |    LZ4_COPYSTEP(ref, op);
      |    ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:417:37: note: in expansion of macro 'A32'
  417 | #define LZ4_COPYSTEP(s, d) A32(d) = A32(s); d += 4; s += 4;
      |                                     ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:418:30: note: in expansion of macro 'LZ4_COPYSTEP'
  418 | #define LZ4_COPYPACKET(s, d) LZ4_COPYSTEP(s, d); LZ4_COPYSTEP(s, d);
      |                              ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:445:36: note: in expansion of macro 'LZ4_COPYPACKET'
  445 | #define LZ4_WILDCOPY(s, d, e) do { LZ4_COPYPACKET(s, d) } while (d < e);
      |                                    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:419:25: note: in expansion of macro 'LZ4_WILDCOPY'
  419 | #define LZ4_SECURECOPY  LZ4_WILDCOPY
      |                         ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:1082:4: note: in expansion of macro 'LZ4_SECURECOPY'
 1082 |    LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
      |    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:417:37: note: in expansion of macro 'A32'
  417 | #define LZ4_COPYSTEP(s, d) A32(d) = A32(s); d += 4; s += 4;
      |                                     ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:418:50: note: in expansion of macro 'LZ4_COPYSTEP'
  418 | #define LZ4_COPYPACKET(s, d) LZ4_COPYSTEP(s, d); LZ4_COPYSTEP(s, d);
      |                                                  ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:445:36: note: in expansion of macro 'LZ4_COPYPACKET'
  445 | #define LZ4_WILDCOPY(s, d, e) do { LZ4_COPYPACKET(s, d) } while (d < e);
      |                                    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:419:25: note: in expansion of macro 'LZ4_WILDCOPY'
  419 | #define LZ4_SECURECOPY  LZ4_WILDCOPY
      |                         ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:1082:4: note: in expansion of macro 'LZ4_SECURECOPY'
 1082 |    LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
      |    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:417:37: note: in expansion of macro 'A32'
  417 | #define LZ4_COPYSTEP(s, d) A32(d) = A32(s); d += 4; s += 4;
      |                                     ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:418:30: note: in expansion of macro 'LZ4_COPYSTEP'
  418 | #define LZ4_COPYPACKET(s, d) LZ4_COPYSTEP(s, d); LZ4_COPYSTEP(s, d);
      |                              ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:445:36: note: in expansion of macro 'LZ4_COPYPACKET'
  445 | #define LZ4_WILDCOPY(s, d, e) do { LZ4_COPYPACKET(s, d) } while (d < e);
      |                                    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:419:25: note: in expansion of macro 'LZ4_WILDCOPY'
  419 | #define LZ4_SECURECOPY  LZ4_WILDCOPY
      |                         ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:1094:3: note: in expansion of macro 'LZ4_SECURECOPY'
 1094 |   LZ4_SECURECOPY(ref, op, cpy);
      |   ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:367:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  367 | #define A32(x) (((U32_S *)(x))->v)
      |                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:417:37: note: in expansion of macro 'A32'
  417 | #define LZ4_COPYSTEP(s, d) A32(d) = A32(s); d += 4; s += 4;
      |                                     ^~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:418:50: note: in expansion of macro 'LZ4_COPYSTEP'
  418 | #define LZ4_COPYPACKET(s, d) LZ4_COPYSTEP(s, d); LZ4_COPYSTEP(s, d);
      |                                                  ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:445:36: note: in expansion of macro 'LZ4_COPYPACKET'
  445 | #define LZ4_WILDCOPY(s, d, e) do { LZ4_COPYPACKET(s, d) } while (d < e);
      |                                    ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:419:25: note: in expansion of macro 'LZ4_WILDCOPY'
  419 | #define LZ4_SECURECOPY  LZ4_WILDCOPY
      |                         ^~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:1094:3: note: in expansion of macro 'LZ4_SECURECOPY'
 1094 |   LZ4_SECURECOPY(ref, op, cpy);
      |   ^~~~~~~~~~~~~~
/code/illumos-gate/usr/src/common/lz4/lz4.c:1103:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
 1103 |  return (int)(-(((char *)ip) - source));
      |                  ^

To address the issues above, we need to be careful about const qualifier - either we need to keep it, or drop. We drop const from A16, A32, A64 macros because those macros are actually used with data we do modify, and that will leave us:

/code/illumos-gate/usr/src/common/lz4/lz4.c: In function 'LZ4_compressCtx':
/code/illumos-gate/usr/src/common/lz4/lz4.c:564:19: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  564 |  const BYTE *ip = (BYTE *) source;
      |                   ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:613:34: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  613 |   while ((ip > anchor) && (ref > (BYTE *) source) &&
      |                                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c: In function 'LZ4_compress64kCtx':
/code/illumos-gate/usr/src/common/lz4/lz4.c:759:19: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  759 |  const BYTE *ip = (BYTE *) source;
      |                   ^
/code/illumos-gate/usr/src/common/lz4/lz4.c:806:34: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
  806 |   while ((ip > anchor) && (ref > (BYTE *) source) &&
      |                                  ^
/code/illumos-gate/usr/src/common/lz4/lz4.c: In function 'LZ4_uncompress_unknownOutputSize':
/code/illumos-gate/usr/src/common/lz4/lz4.c:1103:18: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
 1103 |  return (int)(-(((char *)ip) - source));
      |                  ^

There we need to be sure we do keep const qualifier to fix the warnings above. Those were warnings from userland/loader.

The only thing left is to check over the same -Wextra -Wconst-qual for zfs build:

../../../common/lz4/lz4.c: In function 'lz4_decompress':
../../common/sys/byteorder.h:181:41: error: cast discards 'const' qualifier from pointer target type [-Werror=cast-qual]
  181 | #define BE_IN32(xa) htonl(*((uint32_t *)(void *)(xa)))
      |                                         ^
../../../common/lz4/lz4.c:106:20: note: in expansion of macro 'BE_IN32'
  106 |  uint32_t bufsiz = BE_IN32(src);
      |                    ^~~~~~~

There we can just pass in the function argument s_start. And we only have one warning class left -Wcast-align=strict

../../../common/lz4/lz4.c: In function 'lz4_compress':
../../../common/lz4/lz4.c:92:3: error: cast increases required alignment of target type [-Werror=cast-align]
   92 |  *(uint32_t *)dest = BE_32(bufsiz);
      |   ^
cc1: all warnings being treated as errors

To work around it, we will cast dest (char *) over void *.

Note, none of the fixes above will change the behavior of the lz4 functions.

From other hand, we do drop local instances of lz4.c from loader tree and from ficl/softcore, but those are derivates of the lz4.c from zfs source tree.

Why cleanup with the exact flags above? Because those are the build options I have encountered in illumos and freebsd builds (the same change is being prepared for both illumos and freebsd).

#3

Updated by Electric Monk 18 days ago

  • Status changed from In Progress to Closed
  • % Done changed from 90 to 100

git commit 10ae99ee6a0e5168918d2bba208bcf536edb08f7

commit  10ae99ee6a0e5168918d2bba208bcf536edb08f7
Author: Toomas Soome <tsoome@me.com>
Date:   2019-09-27T18:39:17.000Z

    11667 remove duplicate lz4 implementations
    Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
    Reviewed by: Andy Fiddaman <omnios@citrus-it.co.uk>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF