Bug 101420

Summary:

Function adoptNS and adoptCF should not generate memory leak diagnostic with Clang Static Analyzer

Product:

WebKit

Reporter:

Remy Demarest <rdemarest>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, benjamin, mrowe, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	benjamin: review+, benjamin: commit-queue-
Patch	none

Remy Demarest

Reported 2012-11-06 18:52:14 PST

Function adoptNS and adoptCF should not generate memory leak diagnostic with Clang Static Analyzer <rdar://problem/12644883>

Attachments
Patch (2.93 KB, patch) 2012-11-06 18:56 PST, Remy Demarest	benjamin: review+ benjamin: commit-queue-	Details Formatted Diff Diff
Patch (2.94 KB, patch) 2012-11-07 11:56 PST, Remy Demarest	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Remy Demarest

Comment 1 2012-11-06 18:56:38 PST

Created attachment 172697 [details] Patch

Benjamin Poulain

Comment 2 2012-11-07 01:07:29 PST

Comment on attachment 172697 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172697&action=review Nice patch. I did not know about those annotations. > Source/WTF/ChangeLog:9 > + Defines CF_RELEASES_ARGUMENT and NS_RELEASES_ARGUMENT if not available, uses in function declarations. Ok, not a native English speaker here. :) Should Define and use really take a S? Is the subject implicit or something?

Remy Demarest

Comment 3 2012-11-07 10:50:09 PST

(In reply to comment #2) > (From update of attachment 172697 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=172697&action=review > > Nice patch. I did not know about those annotations. > > > Source/WTF/ChangeLog:9 > > + Defines CF_RELEASES_ARGUMENT and NS_RELEASES_ARGUMENT if not available, uses in function declarations. > > Ok, not a native English speaker here. :) > Should Define and use really take a S? Is the subject implicit or something? Same here, not a native speaker, but if you take the "patch" as the subject of the sentence, then the S makes sense. Though the second sentence might need a "uses them in function declarations".

Benjamin Poulain

Comment 4 2012-11-07 11:48:55 PST

Comment on attachment 172697 [details] Patch > Same here, not a native speaker, but if you take the "patch" as the subject of the sentence, then the S makes sense. Though the second sentence might need a "uses them in function declarations". Okay :) Can you please just update the changelog?

Remy Demarest

Comment 5 2012-11-07 11:56:05 PST

Created attachment 172854 [details] Patch I modified the grammar of the change log to make it more consistent.

Benjamin Poulain

Comment 6 2012-11-07 11:58:20 PST

Comment on attachment 172854 [details] Patch Thank you :)

WebKit Review Bot

Comment 7 2012-11-07 12:35:56 PST

Comment on attachment 172854 [details] Patch Clearing flags on attachment: 172854 Committed r133793: <http://trac.webkit.org/changeset/133793>

WebKit Review Bot

Comment 8 2012-11-07 12:35:59 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.