Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: zstd: check we don't leak symbols; regenerate symbol map #13209

Merged
merged 1 commit into from
Mar 15, 2022

Conversation

nabijaczleweli
Copy link
Contributor

@nabijaczleweli nabijaczleweli commented Mar 13, 2022

Motivation and Context

#12988

Description

This both (a) fixes #12988 and (b) ensures we'll actually catch errors like these on CI in future.

CC: @rincebrain

How Has This Been Tested?

Builds. The new generated compat wrapper included. I reverted it for verification in this checkstyle job: https://github.com/nabijaczleweli/zfs/runs/5528159177?check_suite_focus=true#step:5:34

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the OpenZFS code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied. – CI take my hand
  • All commit messages are properly formatted and contain Signed-off-by.

@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Mar 14, 2022
@behlendorf
Copy link
Contributor

When testing this locally, I got a slightly different ordering of the #define's in zstd_compat_wrapper.h when regenerating. Of course that's not an issue, but just for reference here's the diff from generating on a RHEL 8.5 system.

diff --git a/module/zstd/include/zstd_compat_wrapper.h b/module/zstd/include/zstd_compat_wrapper.h
index 4f81e1c59..ae762f043 100644
--- a/module/zstd/include/zstd_compat_wrapper.h
+++ b/module/zstd/include/zstd_compat_wrapper.h
@@ -172,8 +172,8 @@
 #define        ZSTD_CCtxParams_init zfs_ZSTD_CCtxParams_init
 #define        ZSTD_CCtxParams_init_advanced zfs_ZSTD_CCtxParams_init_advanced
 #define        ZSTD_cParam_getBounds zfs_ZSTD_cParam_getBounds
-#define        ZSTD_CCtxParams_setParameter zfs_ZSTD_CCtxParams_setParameter
 #define        ZSTD_CCtx_setParameter zfs_ZSTD_CCtx_setParameter
+#define        ZSTD_CCtxParams_setParameter zfs_ZSTD_CCtxParams_setParameter
 #define        ZSTD_CCtx_getParameter zfs_ZSTD_CCtx_getParameter
 #define        ZSTD_CCtxParams_getParameter zfs_ZSTD_CCtxParams_getParameter
 #define        ZSTD_CCtx_setParametersUsingCCtxParams zfs_ZSTD_CCtx_setParametersUsingCCtxParams
@@ -219,13 +219,13 @@
 #define        ZSTD_getSequences zfs_ZSTD_getSequences
 #define        ZSTD_createCDict_byReference zfs_ZSTD_createCDict_byReference
 #define        ZSTD_createCDict zfs_ZSTD_createCDict
-#define        ZSTD_compress zfs_ZSTD_compress
 #define        ZSTD_CCtx_refCDict zfs_ZSTD_CCtx_refCDict
-#define        ZSTD_CCtx_refPrefix zfs_ZSTD_CCtx_refPrefix
 #define        ZSTD_CCtx_refPrefix_advanced zfs_ZSTD_CCtx_refPrefix_advanced
+#define        ZSTD_CCtx_refPrefix zfs_ZSTD_CCtx_refPrefix
 #define        ZSTD_CCtx_loadDictionary_byReference zfs_ZSTD_CCtx_loadDictionary_byReference
-#define        ZSTD_freeCCtx zfs_ZSTD_freeCCtx
 #define        ZSTD_CCtx_loadDictionary zfs_ZSTD_CCtx_loadDictionary
+#define        ZSTD_freeCCtx zfs_ZSTD_freeCCtx
+#define        ZSTD_compress zfs_ZSTD_compress
 #define        ZSTD_CCtx_loadDictionary_advanced zfs_ZSTD_CCtx_loadDictionary_advanced
 #define        ZSTD_createCCtx zfs_ZSTD_createCCtx
 #define        ZSTD_createCCtx_advanced zfs_ZSTD_createCCtx_advanced
@@ -308,8 +308,8 @@
 #define        ZSTD_compressBlock_btultra_extDict zfs_ZSTD_compressBlock_btultra_extDict
 
 /* lib/decompress/huf_decompress.o: */
-#define        HUF_readDTableX1_wksp zfs_HUF_readDTableX1_wksp
 #define        HUF_decompress4X1 zfs_HUF_decompress4X1
+#define        HUF_readDTableX1_wksp zfs_HUF_readDTableX1_wksp
 #define        HUF_readDTableX1 zfs_HUF_readDTableX1
 #define        HUF_decompress1X1_usingDTable zfs_HUF_decompress1X1_usingDTable
 #define        HUF_decompress1X1_DCtx_wksp zfs_HUF_decompress1X1_DCtx_wksp
@@ -379,10 +379,10 @@
 #define        ZSTD_loadDEntropy zfs_ZSTD_loadDEntropy
 #define        ZSTD_decompressBegin zfs_ZSTD_decompressBegin
 #define        ZSTD_decompressBegin_usingDict zfs_ZSTD_decompressBegin_usingDict
-#define        ZSTD_decompressBegin_usingDDict zfs_ZSTD_decompressBegin_usingDDict
 #define        ZSTD_decompress_usingDict zfs_ZSTD_decompress_usingDict
 #define        ZSTD_decompressDCtx zfs_ZSTD_decompressDCtx
 #define        ZSTD_decompress zfs_ZSTD_decompress
+#define        ZSTD_decompressBegin_usingDDict zfs_ZSTD_decompressBegin_usingDDict
 #define        ZSTD_getDictID_fromDict zfs_ZSTD_getDictID_fromDict
 #define        ZSTD_getDictID_fromFrame zfs_ZSTD_getDictID_fromFrame
 #define        ZSTD_decompress_usingDDict zfs_ZSTD_decompress_usingDDict

Closes: openzfs#12988
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
@nabijaczleweli
Copy link
Contributor Author

nabijaczleweli commented Mar 14, 2022

Fair cop, made gensymbols sort symbols for each file, so it should be idempotent across regens now.

@behlendorf behlendorf merged commit 6ef0019 into openzfs:master Mar 15, 2022
@PrivatePuffin
Copy link
Contributor

PrivatePuffin commented Apr 3, 2022

Motivation and Context

#12988

Description

This both (a) fixes #12988 and (b) ensures we'll actually catch errors like these on CI in future.

CC: @rincebrain

How Has This Been Tested?

Builds. The new generated compat wrapper included. I reverted it for verification in this checkstyle job: https://github.com/nabijaczleweli/zfs/runs/5528159177?check_suite_focus=true#step:5:34

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the OpenZFS code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied. – CI take my hand
  • All commit messages are properly formatted and contain Signed-off-by.

As a note, it might be handy to also CC @BrainSlayer (that other brain), as he made this code back-in-the-day ;-)

PR was fine though :)

@BrainSlayer
Copy link
Contributor

@Ornias1993 thx for info. i already discovered this feature and used it in my git tree for more recent zstd revisions

veggiemike pushed a commit to veggiemike/zfs that referenced this pull request May 15, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988
Closes openzfs#13209
(cherry picked from commit 6ef0019)
veggiemike pushed a commit to veggiemike/zfs that referenced this pull request May 16, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988
Closes openzfs#13209
(cherry picked from commit 6ef0019)
behlendorf pushed a commit that referenced this pull request May 16, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #12988
Closes #13209
(cherry picked from commit 6ef0019)
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 2, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
beren12 pushed a commit to beren12/zfs that referenced this pull request Sep 19, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Co-authored-by: Rich Ercolani <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12988 
Closes openzfs#13209
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

multiple definition of `HIST_isError' compiling against linux-5.16.1-gentoo kernel
5 participants