Skip to content

Stop using deprecated reset?Stream functions#2504

Merged
Cyan4973 merged 1 commit into
facebook:devfrom
skitt:stop-using-resetxstream
Feb 23, 2021
Merged

Stop using deprecated reset?Stream functions#2504
Cyan4973 merged 1 commit into
facebook:devfrom
skitt:stop-using-resetxstream

Conversation

@skitt

@skitt skitt commented Feb 20, 2021

Copy link
Copy Markdown
Contributor

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
Comment thread tests/zstreamtest.c
}
DISPLAYLEVEL(3, "OK \n");

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

@Cyan4973 Cyan4973 Feb 20, 2021

Copy link
Copy Markdown
Contributor

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

skitt commented Feb 21, 2021

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Contributor

Nope, FreeBSD issues are unrelated.

@Cyan4973 Cyan4973 left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

skitt commented Feb 23, 2021

Copy link
Copy Markdown
Contributor Author

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