Skip to content

Use a single version of mempcpy(3) - #12257

Merged
Girgias merged 4 commits into
php:masterfrom
crrodriguez:zend_mempcpy
Dec 20, 2023
Merged

Use a single version of mempcpy(3)#12257
Girgias merged 4 commits into
php:masterfrom
crrodriguez:zend_mempcpy

Conversation

@crrodriguez

Copy link
Copy Markdown
Contributor

While __php_mempcpy is only used by ext/standard/crypt_sha*, the mempcpy "pattern" is used everywhere.

This commit removes __php_mempcpy, adds zend_mempcpy and transforms open-coded parts into function calls.

While __php_mempcpy is only used by ext/standard/crypt_sha*, the
mempcpy "pattern" is used everywhere.

This commit removes __php_mempcpy, adds zend_mempcpy and transforms
open-coded parts  into function calls.
@alexdowad

Copy link
Copy Markdown
Contributor

No objection for mbstring.

@alexdowad

Copy link
Copy Markdown
Contributor

The failures of the ARM_DEBUG_NTS and FREEBSD_DEBUG_NTS CI tasks are spurious.

@dstogov dstogov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see problems

@TimWolla

Copy link
Copy Markdown
Member

transforms open-coded parts into function calls.

Did you do the replacement manually or did you leverage an automated tool (e.g. Coccinelle)?

@crrodriguez

Copy link
Copy Markdown
Contributor Author

transforms open-coded parts into function calls.

Did you do the replacement manually or did you leverage an automated tool (e.g. Coccinelle)?

Manually reviewed one by one but mostly coccinelle automated (manually it will be very error prone)

@TimWolla

Copy link
Copy Markdown
Member

It might be useful to include your Coccinelle patch in the commit message / PR description for future reference then.

Comment thread configure.ac Outdated
Comment thread ext/mysqlnd/mysqlnd_wireprotocol.c Outdated
Comment thread main/fastcgi.c Outdated
crrodriguez and others added 3 commits December 19, 2023 21:25
Co-authored-by: Gina Peter Banyard <girgias@php.net>
suggestion from Girgias

Co-authored-by: Gina Peter Banyard <girgias@php.net>
Remove line  break

Co-authored-by: Gina Peter Banyard <girgias@php.net>
@crrodriguez

Copy link
Copy Markdown
Contributor Author

Ok, sorry for the delay..I applied all suggestions now.

@Girgias
Girgias merged commit 927adfb into php:master Dec 20, 2023
@Girgias

Girgias commented Dec 20, 2023

Copy link
Copy Markdown
Member

Thank you!

@TimWolla

Copy link
Copy Markdown
Member

@crrodriguez Can you provide your Coccinelle patch as a comment in this PR for posterity?

@crrodriguez

Copy link
Copy Markdown
Contributor Author

@crrodriguez Can you provide your Coccinelle patch as a comment in this PR for posterity?

https://github.com/systemd/systemd/blob/main/coccinelle/mempcpy.cocci

@crrodriguez

Copy link
Copy Markdown
Contributor Author

There are probably other transformations that can be done, that spatch covers the obvious only.
Probably only matters for the readability of the code, gcc handles mempcpy as a builtin, and is simplified to memcpy or assignments or inlined whatever is faster.

adrian-enspired pushed a commit to adrian-enspired/php-src that referenced this pull request Aug 4, 2026
While __php_mempcpy is only used by ext/standard/crypt_sha*, the
mempcpy "pattern" is used everywhere.

This commit removes __php_mempcpy, adds zend_mempcpy and transforms
open-coded parts into function calls.
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.

5 participants