Skip to content

tree-wide: Do not manually check for NULL before calling free() #19063

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

tree-wide: Do not manually check for NULL before calling free() #19063

wants to merge 1 commit into from
+339 −886

Conversation

TimWolla
Copy link
Member

@TimWolla TimWolla commented Jul 7, 2025 •
edited
Loading

Changes performed with Coccinelle:

@@
expression e;
@@

- if (e) {
	efree(e);
- }

@@
expression e;
@@

- if (e) {
	efree(e);
	e = NULL;
- }

@@
expression e;
@@

- if (e) {
	free(e);
- }

@@
expression e;
@@

- if (e) {
	free(e);
	e = NULL;
- }

@@
expression e, e2;
@@

- if (e) {
	pefree(e, e2);
- }

@@
expression e, e2;
@@

- if (e) {
	pefree(e, e2);
	e = NULL;
- }

@TimWolla
tree-wide: Do not manually check for NULL before calling free()
c4097fa 
Changes performed with Coccinelle:

    @@
    expression e;
    @@

    - if (e) {
    	efree(e);
    - }

    @@
    expression e;
    @@

    - if (e) {
    	efree(e);
    	e = NULL;
    - }

    @@
    expression e;
    @@

    - if (e) {
    	free(e);
    - }

    @@
    expression e;
    @@

    - if (e) {
    	free(e);
    	e = NULL;
    - }

    @@
    expression e, e2;
    @@

    - if (e) {
    	pefree(e, e2);
    - }

    @@
    expression e, e2;
    @@

    - if (e) {
    	pefree(e, e2);
    	e = NULL;
    - }
Girgias
Girgias reviewed Jul 7, 2025
View reviewed changes
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Minor CS nits but looks nice :)

ext/snmp/snmp.c
Comment on lines +150 to 151
efree((*session)->a); \
(*session)->a = NULL; \
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
efree((*session)->a); \
(*session)->a = NULL; \
efree((*session)->a); \
(*session)->a = NULL; \

ext/snmp/snmp.c
Comment on lines +363 to +364
/* malloc was used to store value */
efree(dbuf);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/* malloc was used to store value */
efree(dbuf);
/* malloc was used to store value */
efree(dbuf);

Zend/zend_API.c
Comment on lines 1120 to 1122
efree(error);

}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
efree(error);
}
efree(error);
}

@devnexen
Copy link
Member

devnexen commented Jul 7, 2025

the title caught me :) so not just free but zendmm api flavors.

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

@Girgias Girgias Girgias left review comments

@dstogov dstogov Awaiting requested review from dstogov dstogov will be requested when the pull request is marked ready for review dstogov is a code owner

@arnaud-lb arnaud-lb Awaiting requested review from arnaud-lb arnaud-lb will be requested when the pull request is marked ready for review arnaud-lb is a code owner

@nielsdos nielsdos Awaiting requested review from nielsdos nielsdos will be requested when the pull request is marked ready for review nielsdos is a code owner

@bukka bukka Awaiting requested review from bukka bukka will be requested when the pull request is marked ready for review bukka is a code owner

@devnexen devnexen Awaiting requested review from devnexen devnexen will be requested when the pull request is marked ready for review devnexen is a code owner

@SakiTakamachi SakiTakamachi Awaiting requested review from SakiTakamachi SakiTakamachi will be requested when the pull request is marked ready for review SakiTakamachi is a code owner

@NattyNarwhal NattyNarwhal Awaiting requested review from NattyNarwhal NattyNarwhal will be requested when the pull request is marked ready for review NattyNarwhal is a code owner

@kamil-tekiela kamil-tekiela Awaiting requested review from kamil-tekiela kamil-tekiela will be requested when the pull request is marked ready for review kamil-tekiela is a code owner

@alexdowad alexdowad Awaiting requested review from alexdowad alexdowad will be requested when the pull request is marked ready for review alexdowad is a code owner

@youkidearitai youkidearitai Awaiting requested review from youkidearitai youkidearitai will be requested when the pull request is marked ready for review youkidearitai is a code owner

@derickr derickr Awaiting requested review from derickr derickr will be requested when the pull request is marked ready for review derickr is a code owner

Assignees
No one assigned
Labels
ABI break Category: Engine Category: Optimizer Extension: calendar Extension: com_dotnet Extension: date Extension: dba Extension: fileinfo Extension: ftp Extension: gd Extension: hash Extension: intl Extension: mbstring Extension: mysqli Extension: mysqlnd Extension: odbc Extension: opcache Extension: openssl Extension: pdo (core) Extension: pdo_dblib Extension: pdo_firebird Extension: pdo_mysql Extension: pdo_odbc Extension: pdo_pgsql Extension: pdo_sqlite Extension: pgsql Extension: phar Extension: readline Extension: snmp Extension: soap Extension: sockets Extension: sqlite3 Extension: standard Extension: xml Extension: zip Extension: zlib
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TimWolla @devnexen @Girgias