Bug 188337

Summary: [libwebrtc] SafeSetError() in peerconnection.cc contains use-after-move of webrtc::RTCError variable
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: WebRTCAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, commit-queue, eric.carlson, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.chromium.org/p/webrtc/issues/detail?id=9593
Attachments:
Description Flags
Patch v1 none

David Kilzer (:ddkilzer)
Reported 2018-08-05 19:14:00 PDT
SafeSetError() in peerconnection.cc contains use-after-move of webrtc::RTCError variable: bool SafeSetError(webrtc::RTCError error, webrtc::RTCError* error_out) { if (error_out) { *error_out = std::move(error); } return error.ok(); } Seems like error.ok() should be read before the std::move(), or *error_out.ok() or error_out->ok() should be used instead. <rdar://problem/42882908>
Attachments
Patch v1 (3.58 KB, patch)
2018-08-05 19:21 PDT, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 2018-08-05 19:21:07 PDT
Created attachment 346609 [details] Patch v1
Eric Carlson
Comment 2 2018-08-06 05:51:14 PDT
Comment on attachment 346609 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=346609&action=review > Source/ThirdParty/libwebrtc/ChangeLog:4 > + [libwebrtc] SafeSetError() in peerconnection.cc contains use-after-move of webrtc::RTCError variable > + <https://webkit.org/b/188337> Shouldn't this include the bugzilla link?
David Kilzer (:ddkilzer)
Comment 3 2018-08-06 06:35:11 PDT
Comment on attachment 346609 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=346609&action=review >> Source/ThirdParty/libwebrtc/ChangeLog:4 >> + <https://webkit.org/b/188337> > > Shouldn't this include the bugzilla link? That redirects to the bugzilla link. :) I use it because it's shorter.
Eric Carlson
Comment 4 2018-08-06 06:56:39 PDT
Comment on attachment 346609 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=346609&action=review >>> Source/ThirdParty/libwebrtc/ChangeLog:4 >>> + <https://webkit.org/b/188337> >> >> Shouldn't this include the bugzilla link? > > That redirects to the bugzilla link. :) I use it because it's shorter. Ha ha, I meant to say "shouldn't this include the *Radar* link" :-).
David Kilzer (:ddkilzer)
Comment 5 2018-08-06 06:57:54 PDT
Comment on attachment 346609 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=346609&action=review >>>> Source/ThirdParty/libwebrtc/ChangeLog:4 >>>> + <https://webkit.org/b/188337> >>> >>> Shouldn't this include the bugzilla link? >> >> That redirects to the bugzilla link. :) I use it because it's shorter. > > Ha ha, I meant to say "shouldn't this include the *Radar* link" :-). Oops! Will try to stop commit queue.
David Kilzer (:ddkilzer)
Comment 6 2018-08-08 05:07:50 PDT
Committed r234690: <https://trac.webkit.org/changeset/234690>
Note You need to log in before you can comment on or make changes to this bug.