Bug 188337 - [libwebrtc] SafeSetError() in peerconnection.cc contains use-after-move of webrtc::RTCError variable
Summary: [libwebrtc] SafeSetError() in peerconnection.cc contains use-after-move of we...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-05 19:14 PDT by David Kilzer (:ddkilzer)
Modified: 2018-08-08 05:08 PDT (History)
5 users (show)

See Also:

Attachments
Patch v1 (3.58 KB, patch)
2018-08-05 19:21 PDT, David Kilzer (:ddkilzer) 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 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>
Comment 1 David Kilzer (:ddkilzer) 2018-08-05 19:21:07 PDT 
Created attachment 346609 [details]
Patch v1
Comment 2 Eric Carlson 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?
Comment 3 David Kilzer (:ddkilzer) 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.
Comment 4 Eric Carlson 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" :-).
Comment 5 David Kilzer (:ddkilzer) 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.
Comment 6 David Kilzer (:ddkilzer) 2018-08-08 05:07:50 PDT 
Committed r234690: <https://trac.webkit.org/changeset/234690>