Maniphest T48499

delete of NULL pointer is noop, so no need to check
Closed, ResolvedPATCH
Actions

Assigned To
Bastien Montagne (mont29)
Authored By
Lawrence D'Oliveiro (ldo)
May 23 2016, 2:35 AM
Tags
  • BF Blender
Subscribers
Bastien Montagne (mont29)
Lawrence D'Oliveiro (ldo)
Sergey Sharybin (sergey)

Description

In C++, it is unnecessary to check for non-NULL before invoking delete:

if (foo) delete foo;

can be more simply written as

delete foo;

(Ref: section 5.3.5 of the C++98 spec, paragraph 2.)

This patch just fixes some initial cases I came across; no doubt there are others waiting to be found.

0001-delete-of-NULL-pointer-is-noop-so-no-need-to-check.patch2 KBDownload

Related Objects

Mentioned In
rBb6178322dd81: GHOST cleanup (null check before delete, and 0 -> NULL for pointers).
rBa7c3ec4febf2: GHOST cleanup (null check before delete, and 0 -> NULL for pointers).
T48500: Add Explanation About Maintenance Of Extern Libraries
Mentioned Here
T48491: delete of NULL pointer is noop, so no need to check

Event Timeline

Lawrence D'Oliveiro (ldo) created this task.May 23 2016, 2:35 AM
Lawrence D'Oliveiro (ldo) raised the priority of this task from to 90.
Lawrence D'Oliveiro (ldo) updated the task description.
Lawrence D'Oliveiro (ldo) added a project: BF Blender.
Lawrence D'Oliveiro (ldo) edited a custom field.
Lawrence D'Oliveiro (ldo) added a subscriber: Lawrence D'Oliveiro (ldo).
Bastien Montagne (mont29) changed the task status from Unknown Status to Archived.May 23 2016, 9:24 AM
Bastien Montagne (mont29) claimed this task.
Bastien Montagne (mont29) added a subscriber: Bastien Montagne (mont29).

Please do not recreate same task I just closed, this is uselessly rude. I already answered, and this has nothing to do on our tracker anyway. If you want more info/discussion about it, use IRC or ML.

Bastien Montagne (mont29) added a comment.May 23 2016, 9:25 AM

Not to mention this is a totally harmless style issue, nothing actually wrong with this code!

Lawrence D'Oliveiro (ldo) changed the task status from Archived to Unknown Status.May 23 2016, 1:08 PM

This is not the same task.

Bastien Montagne (mont29) changed the task status from Unknown Status to Archived.May 23 2016, 1:13 PM

This is exactly the same task as T48491. Please let task triaging to developpers and stop reopening them and making noise in tracker, we do not need this!

Lawrence D'Oliveiro (ldo) added a comment.May 24 2016, 1:15 AM

Are you saying the GHOST code is off limits for patches as well?

Lawrence D'Oliveiro (ldo) mentioned this in T48500: Add Explanation About Maintenance Of Extern Libraries.May 24 2016, 1:16 AM
Bastien Montagne (mont29) changed the task status from Archived to Unknown Status.May 24 2016, 8:10 AM

sigh… Next time, please mention what your patch is about (what piece of code it affects) in the report… Sorry, but this report looked totally the same as T48491, so no, did not think about looking at patch file itself. GHOST is maintained by us indeed, and totally eligible to patches submission.

About patch itself, looks good, but please try to respect indentation, and replace 0 by NULL for pointer assignments. ;)

Sergey Sharybin (sergey) added a subscriber: Sergey Sharybin (sergey).May 24 2016, 8:58 AM

Something is wrong with indentation. Other than that seems fine.

Sergey Sharybin (sergey) lowered the priority of this task from 90 to Normal.May 24 2016, 11:50 AM

Bastien, are you looking into applying this?

Bastien Montagne (mont29) mentioned this in rBa7c3ec4febf2: GHOST cleanup (null check before delete, and 0 -> NULL for pointers)..May 24 2016, 12:11 PM
Bastien Montagne (mont29) changed the task status from Unknown Status to Resolved.May 24 2016, 12:14 PM
Jeroen Bakker (jbakker) mentioned this in rBb6178322dd81: GHOST cleanup (null check before delete, and 0 -> NULL for pointers)..Jun 8 2016, 9:49 PM