853590 - Remove cprGet/ReleaseMutex from Signaling Code

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Ethan Hugg [:ehugg]]

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.

Comment 1

[Ethan Hugg [:ehugg]]

Assigning to our new Cisco volunteer Charith Tangirala

Comment 2

[Jason Smith [:jsmith]]

This reads off as code cleanup, which would mean this doesn't block.

Comment 3

[Ethan Hugg [:ehugg]]

Yes, this is code cleanup. Things should run exactly the same after this patch.

Comment 4

[Charith Tangirala]

Attachment: Remove unused mutex impl from CPR

Comment 5

[Charith Tangirala]

Attachment: Remove unused mutex impl from CPR

Comment 6

[Ethan Hugg [:ehugg]]

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.

Comment 7

[Charith Tangirala]

Attachment: Remove unused mutex impl from CPR

Comment 8

[Ethan Hugg [:ehugg]]

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

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

Comment 9

[Ethan Hugg [:ehugg]]

Looks like from the try build that we missed a reference to cprCreateMutex in the windows builds.

Comment 10

[Ethan Hugg [:ehugg]]

A quick grep -r of the signaling tree shows cprGet/Release/DeleteMutex are gone but there are still many instances of cprCreateMutex

Comment 11

[Charith Tangirala]

Attachment: Remove unused mutex impl from CPR

Comment 12

[Ethan Hugg [:ehugg]]

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.

Comment 13

[Charith Tangirala]

Attachment: Remove unused mutex impl from CPR

Comment 14

[Ethan Hugg [:ehugg]]

Try run with new patch - https://tbpl.mozilla.org/?tree=Try&rev=6fbc4904027c

Comment 15

[Ethan Hugg [:ehugg]]

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


Try run is green.  Requesting review from Randell Jesup

Comment 16

[Ethan Hugg [:ehugg]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5ec9994badfc

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5ec9994badfc