Skip to content

Switch to smart_str API#95

Merged
kjdev merged 1 commit into
kjdev:masterfrom
ndossche:smart-str
May 27, 2026
Merged

Switch to smart_str API#95
kjdev merged 1 commit into
kjdev:masterfrom
ndossche:smart-str

Conversation

@ndossche
Copy link
Copy Markdown
Contributor

@ndossche ndossche commented May 26, 2026

This is the same as kjdev/php-ext-brotli#83, but for zstd.

Summary by CodeRabbit

  • Refactor
    • Updated the ZSTD extension implementation to use modern PHP APIs for improved code quality and maintainability.
    • Enhanced compatibility support for older PHP versions while maintaining support for newer versions.

Review Change Stack

This is the same as kjdev/php-ext-brotli#83, but
for zstd.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6bf34dc7-264e-4373-82ba-f911c77b3039

📥 Commits

Reviewing files that changed from the base of the PR and between d6d26eb and 05f5d05.

📒 Files selected for processing (1)
  • zstd.c

📝 Walkthrough

Walkthrough

This PR upgrades the PHP ZSTD extension to use the modern smart_str API instead of the deprecated smart_string buffer type. A compatibility helper was added for older PHP versions, header includes were reordered, and all compression/decompression functions (one-shot and incremental) were updated to initialize, append to, free, and extract results from smart_str buffers.

Changes

ZSTD smart_str API migration

Layer / File(s) Summary
Infrastructure and compatibility setup
zstd.c
Headers reordered to include zend_smart_str.h before APCu conditional blocks. Added inline smart_str_extract() helper guarded by PHP_VERSION_ID < 70200 to safely extract the owned zend_string from a smart_str buffer or return an empty allocated string for older PHP versions.
Compression functions: buffer and return migration
zstd.c
Updated zstd_compress, zstd_compress_dict, and zstd_compress_add to replace smart_string with smart_str for output accumulation. Error paths now free the smart_str, output is appended via smart_str_appendl, and functions return via RETVAL_STR(smart_str_extract(&out)).
Decompression functions: buffer and return migration
zstd.c
Updated zstd_uncompress, zstd_uncompress_dict, and zstd_uncompress_add to replace smart_string with smart_str for output accumulation. Error paths free the smart_str and emit appropriate warnings, output is appended via smart_str_appendl, and functions return via RETVAL_STR(smart_str_extract(&out)).

Possibly related PRs

  • kjdev/php-ext-zstd#86: Both PRs refactor the same output buffering logic in zstd.c to use PHP smart-string APIs; this PR switches to the newer smart_str with a compatibility helper, while #86 rewrites the same functions using smart_string.
  • kjdev/php-ext-zstd#88: Both PRs modify header inclusion and add compatibility for different PHP versions to support the smart_str/zend_smart_str API at the same integration points.
  • kjdev/php-ext-zstd#79: The incremental compression/decompression functions updated in this PR (zstd_compress_add, zstd_uncompress_add) are the same functions introduced in #79, so the smart_str migration directly applies to that feature.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 The buffers, they whispered of times long past,
So we swapped out the old for the new built to last,
With smart_str extracted and compatibility tight,
Compression flows onward—hooray! All is right! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Switch to smart_str API' accurately and concisely describes the primary change in the changeset, which updates the PHP ZSTD extension to use the smart_str API instead of the older smart_string API.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kjdev kjdev merged commit 5942b5d into kjdev:master May 27, 2026
111 checks passed
@kjdev
Copy link
Copy Markdown
Owner

kjdev commented May 27, 2026

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants