Skip to content

ext/zip: use php_globfree wrapper in addGlob early returns#21709

Closed
iliaal wants to merge 1 commit intophp:PHP-8.5from
iliaal:fix/zip-globfree-wrapper
Closed

ext/zip: use php_globfree wrapper in addGlob early returns#21709
iliaal wants to merge 1 commit intophp:PHP-8.5from
iliaal:fix/zip-globfree-wrapper

Conversation

@iliaal
Copy link
Copy Markdown
Contributor

@iliaal iliaal commented Apr 10, 2026

Master fails to build on ALPINE_X64_ASAN_DEBUG_ZTS, LINUX_X64_ASAN_DEBUG_ZTS, LINUX_X64_RELEASE_NTS, MACOS_ARM64_DEBUG_NTS, and FREEBSD_NTS with:

ext/zip/php_zip.c:679:17: error: implicit declaration of function 'globfree'; did you mean 'php_globfree'? [-Werror=implicit-function-declaration]

GH-21702 added two globfree(&globbuf) calls at ext/zip/php_zip.c:679 and :686 for the no-match and open_basedir reject paths. I switched both to php_globfree(), matching the existing call at line 715 and the main/php_glob.h convention.

Two-line change. Fixes the build on affected platforms without touching behavior.

@devnexen
Copy link
Copy Markdown
Member

ah you were faster but please target PHP-8.5

phpGH-21702 added two `globfree()` calls at the no-match and open_basedir
reject paths, but called `globfree` directly instead of the `php_globfree`
wrapper used at the success path below. PHP-8.5 dropped the direct
`<glob.h>` include from `ext/zip/php_zip.c` in favor of the `php_glob.h`
wrapper, so the build now breaks with
`-Werror=implicit-function-declaration` on systems where `<glob.h>` isn't
transitively included.

Match the existing wrapper usage at line 675.
@iliaal
Copy link
Copy Markdown
Contributor Author

iliaal commented Apr 10, 2026

ah you were faster but please target PHP-8.5

Only because I was watching my PR break, thinking I broke something 😅

Rebased on 8.5.

@devnexen devnexen closed this in 556757d Apr 10, 2026
@devnexen
Copy link
Copy Markdown
Member

Thanks a mil !

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.

2 participants