Remove cprGet/ReleaseMutex from Signaling Code

RESOLVED FIXED in mozilla23

Status

()

Core
WebRTC: Signaling
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: ehugg, Assigned: Charith Tangirala)

Tracking

Trunk
mozilla23
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [WebRTC][blocking-webrtc-][qa-])

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

5 years ago
Signaling code still has instances of cprGet/ReleaseMutex in kpmlmap.c and is defined in cpr for all OS targets.  These functions should be removed entirely.  The instances in kpml can be either replaces with mozilla/Mutex or the kpml functions can be stubbed out since we don't currently support kpml.
(Reporter)

Comment 1

5 years ago
Assigning to our new Cisco volunteer Charith Tangirala
Assignee: nobody → ctangira
This reads off as code cleanup, which would mean this doesn't block.
Whiteboard: [WebRTC][blocking-webrtc-]
(Reporter)

Comment 3

5 years ago
Yes, this is code cleanup. Things should run exactly the same after this patch.
(Assignee)

Comment 4

4 years ago
Created attachment 731188 [details] [diff] [review]
Remove unused mutex impl from CPR
(Assignee)

Comment 5

4 years ago
Created attachment 731192 [details] [diff] [review]
Remove unused mutex impl from CPR
(Assignee)

Updated

4 years ago
Attachment #731188 - Attachment is obsolete: true
(Reporter)

Comment 6

4 years ago
Comment on attachment 731192 [details] [diff] [review]
Remove unused mutex impl from CPR

Review of attachment 731192 [details] [diff] [review]:
-----------------------------------------------------------------

Unless there's a reason to keep cprCreateMutex and cprDestroyMutex you should remove those as well.  Update with this patch applied, then do an 'hg qref' and another bzexport.  If it looks OK, then I'll run a Try for you.

::: media/webrtc/signaling/src/sipcc/cpr/win32/cpr_win_locks.c
@@ +72,3 @@
>      cprMutexPtr = (cpr_mutex_t *) cpr_malloc(sizeof(cpr_mutex_t));
>      if (cprMutexPtr != NULL) {
> +         Assign name if one was passed in

These changes look suspicious, did you mean to remove the comment chars?  I'm guessing you meant to remove these altogether.
(Assignee)

Comment 7

4 years ago
Created attachment 731242 [details] [diff] [review]
Remove unused mutex impl from CPR
(Assignee)

Updated

4 years ago
Attachment #731192 - Attachment is obsolete: true
(Reporter)

Comment 8

4 years ago
Pushed to try - https://tbpl.mozilla.org/?tree=Try&rev=25410da1cccf

If this builds green, I'll ask for review probably from Jesup.
(Reporter)

Comment 9

4 years ago
Looks like from the try build that we missed a reference to cprCreateMutex in the windows builds.
(Reporter)

Comment 10

4 years ago
A quick grep -r of the signaling tree shows cprGet/Release/DeleteMutex are gone but there are still many instances of cprCreateMutex
(Assignee)

Comment 11

4 years ago
Created attachment 732011 [details] [diff] [review]
Remove unused mutex impl from CPR
(Assignee)

Updated

4 years ago
Attachment #731242 - Attachment is obsolete: true
(Reporter)

Comment 12

4 years ago
Got this on my Linux box with this patch:

/home/ehugg/mozilla/mozilla-central/media/webrtc/signaling/src/sipcc/cpr/linux/cpr_linux_init.c:116:29: fatal error: cpr_linux_locks.h: No such file or directory
compilation terminated.

Please 'grep -r cpr_linux_locks.h' the signaling directory for each file you removed to find any more.
(Assignee)

Comment 13

4 years ago
Created attachment 732097 [details] [diff] [review]
Remove unused mutex impl from CPR
(Assignee)

Updated

4 years ago
Attachment #732011 - Attachment is obsolete: true
(Reporter)

Comment 14

4 years ago
Try run with new patch - https://tbpl.mozilla.org/?tree=Try&rev=6fbc4904027c
(Reporter)

Comment 15

4 years ago
Comment on attachment 732097 [details] [diff] [review]
Remove unused mutex impl from CPR


Try run is green.  Requesting review from Randell Jesup
Attachment #732097 - Flags: review?(rjesup)

Updated

4 years ago
Attachment #732097 - Flags: review?(rjesup) → review+
(Reporter)

Comment 16

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ec9994badfc
https://hg.mozilla.org/mozilla-central/rev/5ec9994badfc
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23

Updated

4 years ago
Whiteboard: [WebRTC][blocking-webrtc-] → [WebRTC][blocking-webrtc-][qa-]
You need to log in before you can comment on or make changes to this bug.