Bug 42464 - Really add WARN_UNUSED_RESULT to leakRef
Summary: Really add WARN_UNUSED_RESULT to leakRef
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-16 09:52 PDT by Anders Carlsson
Modified: 2010-07-16 12:43 PDT (History)
4 users (show)

See Also:


Attachments
Patch (5.31 KB, patch)
2010-07-16 10:00 PDT, Anders Carlsson
levin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2010-07-16 09:52:23 PDT
Really add WARN_UNUSED_RESULT to leakRef and leakPtr
Comment 1 Anders Carlsson 2010-07-16 10:00:22 PDT
Created attachment 61819 [details]
Patch
Comment 2 WebKit Review Bot 2010-07-16 10:01:34 PDT
Attachment 61819 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
JavaScriptCore/wtf/PassRefPtr.h:155:  More than one command on the same line  [whitespace/newline] [4]
JavaScriptCore/wtf/RetainPtr.h:74:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 David Levin 2010-07-16 10:21:47 PDT
Comment on attachment 61819 [details]
Patch

> diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
> +        * wtf/RetainPtr.h:
> +        (WTF::RetainPtr::releaseRef):
> +        Remove WARN_UNUSED_RESULT here for now, it leads to two warnings that need
> +        to be fixed first.

fwiw, you appear to only have fixed one of them.
Comment 4 Anders Carlsson 2010-07-16 10:28:53 PDT
(In reply to comment #3)
> (From update of attachment 61819 [details])
> > diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
> > +        * wtf/RetainPtr.h:
> > +        (WTF::RetainPtr::releaseRef):
> > +        Remove WARN_UNUSED_RESULT here for now, it leads to two warnings that need
> > +        to be fixed first.
> 
> fwiw, you appear to only have fixed one of them.

I fixed the single RefPtr leakRef warning. There are two RetainPtr leakPtr calls that I would need to fix first before adding back WARN_UNUSED_RESULT on RetainPtr.
Comment 5 Anders Carlsson 2010-07-16 10:47:20 PDT
Committed r63562: <http://trac.webkit.org/changeset/63562>
Comment 6 David Levin 2010-07-16 10:49:33 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 61819 [details] [details])
> > > diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
> > > +        * wtf/RetainPtr.h:
> > > +        (WTF::RetainPtr::releaseRef):
> > > +        Remove WARN_UNUSED_RESULT here for now, it leads to two warnings that need
> > > +        to be fixed first.
> > 
> > fwiw, you appear to only have fixed one of them.
> 
> I fixed the single RefPtr leakRef warning. There are two RetainPtr leakPtr calls that I would need to fix first before adding back WARN_UNUSED_RESULT on RetainPtr.

Of course, you said it right there. Sorry about that.
Comment 7 WebKit Review Bot 2010-07-16 12:43:07 PDT
http://trac.webkit.org/changeset/63562 might have broken Chromium Win Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/63561
http://trac.webkit.org/changeset/63562
http://trac.webkit.org/changeset/63563