Page MenuHomeFreeBSD

Remove NULL check before free(9)
AbandonedPublic

Authored by howard0su_gmail.com on Apr 11 2016, 5:28 AM.
Tags
None
Referenced Files
F82022204: D5904.diff
Wed, Apr 24, 4:32 PM
Unknown Object (File)
Feb 22 2024, 8:08 PM
Unknown Object (File)
Feb 9 2024, 5:02 PM
Unknown Object (File)
Jan 18 2024, 12:29 AM
Unknown Object (File)
Dec 22 2023, 11:29 PM
Unknown Object (File)
Dec 8 2023, 7:09 AM
Unknown Object (File)
Dec 8 2023, 7:06 AM
Unknown Object (File)
Nov 22 2023, 8:49 PM

Details

Reviewers
cem
jmg
gnn
sbruno
Group Reviewers
Intel Networking
Summary

Use the following script of Conccinelle:
@@
expression E;
expression X;
@@

-if (E != NULL) {

free(E, X);
...

-}

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

howard0su_gmail.com retitled this revision from to Remove NULL check before free(9).
howard0su_gmail.com updated this object.
howard0su_gmail.com edited the test plan for this revision. (Show Details)
howard0su_gmail.com set the repository for this revision to rS FreeBSD src repository - subversion.
allanjude added inline comments.
sys/dev/acpica/acpi_thermal.c
961

Indent appears wrong here.

sys/dev/atkbdc/atkbd.c
459

the script seems to have missed some of the other calls to free() here

sys/dev/ciss/ciss.c
1287

indent

1782

indent

sys/dev/drm2/i915/dvo_ch7017.c
404

Not sure we want to mess with the i915 files where we are trying to stay in sync with Linux

cem requested changes to this revision.Apr 11 2016, 6:01 AM
cem added a reviewer: cem.
cem added a subscriber: cem.

I don't think this transformation is correct. It's not valid to lift everything out of the if(){}, only the free() itself. And if there are other statements in the if(){}, I don't believe this improves clarity.

Like Allan pointed out, drm2/i915 is mostly contrib code from Linux; I'd prefer to leave that alone to ease porting efforts.

sys/dev/drm2/i915/dvo_ch7017.c
404

Agreed, and this transformation seems wrong anyway. Only the free can be lifted out of the if(){}.

This revision now requires changes to proceed.Apr 11 2016, 6:01 AM

The patch is still refining. I submit this to see if there is strong objection not to do this. (remove null check)

sys/dev/drm2/i915/dvo_ch7017.c
404

I will skip drm code and code under contrib.

For this case, I still believe the logic is correct. dvo->dev_priv was pointing to priv.
priv = malloc(sizeof(struct ch7017_priv), DRM_MEM_KMS, M_NOWAIT | M_ZERO);
if (priv == NULL)

return false;

dvo->i2c_bus = adapter;

-------dvo->dev_priv = priv;

Some of these files, for example firewire and reiserfs, are not built by default. Make sure you compile test them with a suitable kernel config file.

sys/crypto/via/padlock_hash.c
188

Whitespace bug

sys/dev/iscsi_initiator/isc_subr.c
258

Whitespace bug

sys/dev/iscsi_initiator/iscsi_subr.c
330

Whitespace bug

337

Whitespace bug here too

sys/dev/pms/freebsd/driver/common/lxutil.c
693

This looks suspect. Will pCardInfo->tiCacheMem[idx] commonly already be NULL? If so, then your change will DOS the system console. Without knowing how lxutil works or hearing from lxutil's maintainer, I don't think you should change it.

sys/kern/kern_jail.c
2759

Did you mean to add the newline here and on line 3098?

sys/netpfil/ipfw/ip_dummynet.c
1249

whitespace bug

I have to split this into small changeset and manually cleanup more stlye(9)