Skip to content

Fix zstd:flush/2#10930

Open
rickard-green wants to merge 1 commit intoerlang:masterfrom
rickard-green:rickard/zstd-flush-fix
Open

Fix zstd:flush/2#10930
rickard-green wants to merge 1 commit intoerlang:masterfrom
rickard-green:rickard/zstd-flush-fix

Conversation

@rickard-green
Copy link
Copy Markdown
Contributor

Fix zstd:flush/2 introduced in #10511 @essen @jhogberg

In debug mode, the zstd NIF reduce the chunk size to 5 which causes the zstd:flush/2 function in the cstream testcase to return {flush, _} which causes the testcase to fail. {flush, _} is not a valid return value according to the docs.

This PR prevents the testcase failure, but the change in this PR is a guess of what the flush/2 function should have done. I'm not sure if this fix is correct or complete.

@rickard-green rickard-green added team:VM Assigned to OTP team VM fix labels Mar 27, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

CT Test Results

    2 files    100 suites   1h 7m 15s ⏱️
2 288 tests 2 236 ✅ 52 💤 0 ❌
2 706 runs  2 650 ✅ 56 💤 0 ❌

Results for commit 8413ea4.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

Comment thread lib/stdlib/src/zstd.erl
Res ->
{flush, Output} ->
{_, Output2} = flush(Ctx, <<>>),
{continue, [Output | Output2]};
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.

This violates the spec too, the output has to be {continue, iovec(), binary()} or {continue, binary()}, we can't mix it up.

Suggested change
{continue, [Output | Output2]};
{continue, iolist_to_binary([Output, Output2])};

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.

What happens if that output is sent directly to a file/socket, isn't the iolist_to_binary unnecessary there? Perhaps zstd can be updated to allow it to return more than binary() output?

@jhogberg jhogberg self-assigned this Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix team:VM Assigned to OTP team VM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants