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

Stop using deprecated reset?Stream functions #2504

Merged
merged 1 commit into from
Feb 23, 2021

Conversation

skitt
Copy link
Contributor

@skitt skitt commented Feb 20, 2021

These are replaced by the corresponding context resets. When
converting resetCStream, setPledgedSrcSize isn't called if the source
size is "unknown".

This helps reduce the reliance on "static only" symbols, as well as
reducing the use of deprecated functions.

Signed-off-by: Stephen Kitt steve@sk2.org

@skitt skitt marked this pull request as draft February 20, 2021 16:33
@skitt skitt force-pushed the stop-using-resetxstream branch from 98bc5e2 to 2fdc883 Compare February 20, 2021 16:35
@@ -1071,19 +1071,19 @@ static int basicUnitTests(U32 seed, double compressibility)
}
DISPLAYLEVEL(3, "OK \n");

DISPLAYLEVEL(3, "test%3i : ZSTD_resetDStream() with dictionary : ", testNb++);
Copy link
Contributor

@Cyan4973 Cyan4973 Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend not modifying the test functions,
such as the ones included in tests/zstreamtest.c
which scope includes checking static symbols.

@skitt skitt force-pushed the stop-using-resetxstream branch from 2fdc883 to 7d9059a Compare February 20, 2021 20:03
@skitt skitt marked this pull request as ready for review February 21, 2021 19:48
@skitt
Copy link
Contributor Author

skitt commented Feb 21, 2021

I don’t think the FreeBSD failure is related to this patch:

ld-elf.so.1: /usr/local/bin/gmd5sum: Undefined symbol "fread_unlocked@FBSD_1.6"

@Cyan4973
Copy link
Contributor

Nope, FreeBSD issues are unrelated.

Copy link
Contributor

@Cyan4973 Cyan4973 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with the proposed changes.

I would wait for @terrelln 's feedback regarding changes in the linux-kernel and pzstd projects.

@terrelln
Copy link
Contributor

The changes to pzstd looks good.

I'd like to revert the changes to linux-kernel. It isn't used in user-space, so it doesn't impact the static symbol usage. And I'd rather keep the implementations in the module.c files straightforward.

These are replaced by the corresponding context resets. When
converting resetCStream, CCtx_setPledgedSrcSize isn't called if the
source size is "unknown".

This helps reduce the reliance on "static only" symbols, as well as
reducing the use of deprecated functions.

Signed-off-by: Stephen Kitt <steve@sk2.org>
@skitt skitt force-pushed the stop-using-resetxstream branch from 7d9059a to adb5429 Compare February 23, 2021 20:56
@skitt
Copy link
Contributor Author

skitt commented Feb 23, 2021

I'd like to revert the changes to linux-kernel. It isn't used in user-space, so it doesn't impact the static symbol usage. And I'd rather keep the implementations in the module.c files straightforward.

Done, thanks for the review!

@Cyan4973 Cyan4973 merged commit d1c0ae9 into facebook:dev Feb 23, 2021
@skitt skitt deleted the stop-using-resetxstream branch February 23, 2021 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants