Add WARN_UNUSED_RETURN to decode methods in Source/WTF. $ ./Tools/Scripts/check-webkit-style --filter=-,+security/missing_warn_unused_return Source/WTF ERROR: Source/WTF/wtf/URL.h:195: decode() function returning a value is missing WARN_UNUSED_RETURN attribute [security/missing_warn_unused_return] [5] ERROR: Source/WTF/wtf/URL.h:196: decode() function returning a value is missing WARN_UNUSED_RETURN attribute [security/missing_warn_unused_return] [5] ERROR: Source/WTF/wtf/MediaTime.h:138: decode() function returning a value is missing WARN_UNUSED_RETURN attribute [security/missing_warn_unused_return] [5] ERROR: Source/WTF/wtf/Seconds.h:232: decode() function returning a value is missing WARN_UNUSED_RETURN attribute [security/missing_warn_unused_return] [5] ERROR: Source/WTF/wtf/Seconds.h:242: decode() function returning a value is missing WARN_UNUSED_RETURN attribute [security/missing_warn_unused_return] [5] ERROR: Source/WTF/wtf/MonotonicTime.h:149: decode() function returning a value is missing WARN_UNUSED_RETURN attribute [security/missing_warn_unused_return] [5] ERROR: Source/WTF/wtf/MonotonicTime.h:159: decode() function returning a value is missing WARN_UNUSED_RETURN attribute [security/missing_warn_unused_return] [5] ERROR: Source/WTF/wtf/ObjectIdentifier.h:73: decode() function returning a value is missing WARN_UNUSED_RETURN attribute [security/missing_warn_unused_return] [5] ERROR: Source/WTF/wtf/persistence/PersistentCoder.h:41: decode() function returning a value is missing WARN_UNUSED_RETURN attribute [security/missing_warn_unused_return] [5] Total errors found: 9 in 819 files
<rdar://problem/61565764>
No issues found. This is to prevent future bugs from being introduced.
Created attachment 396068 [details] Patch v1
Comment on attachment 396068 [details] Patch v1 For the functions that return Optional, WARN_UNUSED_RETURN has far less utility than for the functions that return bool. Not even sure it’s worthwhile.
(In reply to Darin Adler from comment #4) > Comment on attachment 396068 [details] > Patch v1 > > For the functions that return Optional, WARN_UNUSED_RETURN has far less > utility than for the functions that return bool. Not even sure it’s > worthwhile. I have yet to find any cases where the Optional<> values are not already checked, though. (Maybe that's because the output is always assigned to another Optional<> variable?) I'll take a look at how they're used.
(In reply to David Kilzer (:ddkilzer) from comment #5) > I have yet to find any cases where the Optional<> values are not already > checked, though. (Maybe that's because the output is always assigned to > another Optional<> variable?) My point exactly. It’s very hard to make the mistake of not using the Optional<> because of the design of Optional. It’s very easy to make the mistake of not using the bool, which is why this is important.
Committed r259878: <https://trac.webkit.org/changeset/259878> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396068 [details].
I don't think we should add WARN_UNUSED_RETURN to decoders returning Optional. I also don't think we should go through effort to add WARN_UNUSED_RETURN to existing decoders that return a bool. That effort should be used to make decoders that return Optional instead.
(In reply to Alex Christensen from comment #8) > I don't think we should add WARN_UNUSED_RETURN to decoders returning > Optional. I agree. > I also don't think we should go through effort to add WARN_UNUSED_RETURN to > existing decoders that return a bool. That effort should be used to make > decoders that return Optional instead. I don’t agree. The effort to refactor from bool to Optional is greater than the effort to add the warning. And the warning protects us until we refactor.
Could we at least land https://bugs.webkit.org/show_bug.cgi?id=207497 before doing much more IPC/encoding stuff?
(In reply to Alex Christensen from comment #10) > Could we at least land https://bugs.webkit.org/show_bug.cgi?id=207497 before > doing much more IPC/encoding stuff? Sure.
(In reply to David Kilzer (:ddkilzer) from comment #11) > (In reply to Alex Christensen from comment #10) > > Could we at least land https://bugs.webkit.org/show_bug.cgi?id=207497 before > > doing much more IPC/encoding stuff? > > Sure. Except I just landed this follow-up fix for the suggestions you and Darin made: Committed r259917: <https://trac.webkit.org/changeset/259917> Sorry about that. Didn't see this until after I landed the change.