From 5d015647a68892f2a776115ae31eab97e74c87b2 Mon Sep 17 00:00:00 2001 From: "Adam Roach [:abr]" Date: Mon, 31 Dec 2012 11:43:22 -0600 Subject: [PATCH] Bug 821071: Initialize all out parameters in VcmSIPCCBinding.cpp, r=ekr --- .../signaling/src/media/VcmSIPCCBinding.cpp | 138 ++++++++++-------- .../signaling/src/sipcc/core/gsm/fsmdef.c | 12 +- .../webrtc/signaling/src/sipcc/core/gsm/lsm.c | 12 +- .../webrtc/signaling/src/sipcc/include/vcm.h | 16 +- 4 files changed, 103 insertions(+), 75 deletions(-) diff --git a/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp b/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp index d8a17bdfd0c6..dbe23423c304 100644 --- a/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp +++ b/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp @@ -369,6 +369,7 @@ void vcmRxAllocPort(cc_mcapid_t mcap_id, cc_uint16_t port_requested, int *port_allocated) { + *port_allocated = -1; CSFLogDebug( logTag, "vcmRxAllocPort(): group_id=%d stream_id=%d call_handle=%d port_requested = %d", group_id, stream_id, call_handle, port_requested); @@ -410,22 +411,25 @@ void vcmRxAllocPort(cc_mcapid_t mcap_id, * @param[out] candidatesp - the ICE candidate array * @param[out] candidate_ctp length of the array * - * @return void + * @return 0 for success; VCM_ERROR for failure * */ -static void vcmRxAllocICE_m(cc_mcapid_t mcap_id, - cc_groupid_t group_id, - cc_streamid_t stream_id, - cc_call_handle_t call_handle, - const char *peerconnection, - uint16_t level, - char **default_addrp, /* Out */ - int *default_portp, /* Out */ - char ***candidatesp, /* Out */ - int *candidate_ctp /* Out */ +static short vcmRxAllocICE_m(cc_mcapid_t mcap_id, + cc_groupid_t group_id, + cc_streamid_t stream_id, + cc_call_handle_t call_handle, + const char *peerconnection, + uint16_t level, + char **default_addrp, /* Out */ + int *default_portp, /* Out */ + char ***candidatesp, /* Out */ + int *candidate_ctp /* Out */ ) { + *default_addrp = NULL; *default_portp = -1; + *candidatesp = NULL; + *candidate_ctp = 0; CSFLogDebug( logTag, "%s: group_id=%d stream_id=%d call_handle=%d PC = %s", __FUNCTION__, group_id, stream_id, call_handle, peerconnection); @@ -434,14 +438,14 @@ static void vcmRxAllocICE_m(cc_mcapid_t mcap_id, // ICE streams already exist, so we're just acquiring them. Any logic // to make them on demand is elsewhere. sipcc::PeerConnectionWrapper pc(peerconnection); - ENSURE_PC(pc, /**/); + ENSURE_PC(pc, VCM_ERROR); CSFLogDebug( logTag, "%s: Getting stream %d", __FUNCTION__, level); mozilla::RefPtr stream = pc.impl()->media()-> ice_media_stream(level-1); MOZ_ASSERT(stream); if (!stream) { - return; + return VCM_ERROR; } std::vector candidates = stream->GetCandidates(); @@ -453,7 +457,7 @@ static void vcmRxAllocICE_m(cc_mcapid_t mcap_id, nsresult res = stream->GetDefaultCandidate(1, &default_addr, &default_port); MOZ_ASSERT(NS_SUCCEEDED(res)); if (!NS_SUCCEEDED(res)) { - return; + return VCM_ERROR; } CSFLogDebug( logTag, "%s: Got default candidates %s:%d", __FUNCTION__, @@ -462,7 +466,7 @@ static void vcmRxAllocICE_m(cc_mcapid_t mcap_id, // Note: this leaks memory if we are out of memory. Oh well. *candidatesp = (char **) cpr_malloc(candidates.size() * sizeof(char *)); if (!(*candidatesp)) - return; + return VCM_ERROR; for (size_t i=0; iDispatch( - WrapRunnableNM(&vcmRxAllocICE_m, - mcap_id, - group_id, - stream_id, - call_handle, - peerconnection, - level, - default_addrp, - default_portp, - candidatesp, - candidate_ctp), - NS_DISPATCH_SYNC); + WrapRunnableNMRet(&vcmRxAllocICE_m, + mcap_id, + group_id, + stream_id, + call_handle, + peerconnection, + level, + default_addrp, + default_portp, + candidatesp, + candidate_ctp, + &ret), + NS_DISPATCH_SYNC); + return ret; } /* Get ICE global parameters (ufrag and pwd) @@ -528,11 +536,11 @@ void vcmRxAllocICE(cc_mcapid_t mcap_id, * @param[out] ufragp - where to put the ufrag * @param[out] pwdp - where to put the pwd * - * @return void + * @return 0 for success; VCM_ERROR for failure */ -static void vcmGetIceParams_m(const char *peerconnection, - char **ufragp, - char **pwdp) +static short vcmGetIceParams_m(const char *peerconnection, + char **ufragp, + char **pwdp) { CSFLogDebug( logTag, "%s: PC = %s", __FUNCTION__, peerconnection); @@ -542,7 +550,7 @@ static void vcmGetIceParams_m(const char *peerconnection, // ICE streams already exist, so we're just acquiring them. Any logic // to make them on demand is elsewhere. sipcc::PeerConnectionWrapper pc(peerconnection); - ENSURE_PC(pc, /**/); + ENSURE_PC(pc, VCM_ERROR); std::vector attrs = pc.impl()->media()-> ice_ctx()->GetGlobalAttributes(); @@ -556,7 +564,7 @@ static void vcmGetIceParams_m(const char *peerconnection, if (!ufrag) { ufrag = (char *) cpr_malloc(attrs[i].size() + 1); if (!ufrag) - return; + return VCM_ERROR; sstrncpy(ufrag, attrs[i].c_str(), attrs[i].size() + 1); ufrag[attrs[i].size()] = 0; } @@ -565,7 +573,7 @@ static void vcmGetIceParams_m(const char *peerconnection, if (attrs[i].compare(0, strlen("ice-pwd:"), "ice-pwd:") == 0) { pwd = (char *) cpr_malloc(attrs[i].size() + 1); if (!pwd) - return; + return VCM_ERROR; sstrncpy(pwd, attrs[i].c_str(), attrs[i].size() + 1); pwd[attrs[i].size()] = 0; } @@ -576,13 +584,13 @@ static void vcmGetIceParams_m(const char *peerconnection, cpr_free(ufrag); cpr_free(pwd); CSFLogDebug( logTag, "%s: no ufrag or password", __FUNCTION__); - return; + return VCM_ERROR; } *ufragp = ufrag; *pwdp = pwd; - return; + return 0; } /* Get ICE global parameters (ufrag and pwd) @@ -593,18 +601,21 @@ static void vcmGetIceParams_m(const char *peerconnection, * @param[out] ufragp - where to put the ufrag * @param[out] pwdp - where to put the pwd * - * @return void + * @return 0 for success; VCM_ERROR for failure */ -void vcmGetIceParams(const char *peerconnection, +short vcmGetIceParams(const char *peerconnection, char **ufragp, char **pwdp) { + int ret; VcmSIPCCBinding::getMainThread()->Dispatch( - WrapRunnableNM(&vcmGetIceParams_m, - peerconnection, - ufragp, - pwdp), + WrapRunnableNMRet(&vcmGetIceParams_m, + peerconnection, + ufragp, + pwdp, + &ret), NS_DISPATCH_SYNC); + return ret; } @@ -614,7 +625,7 @@ void vcmGetIceParams(const char *peerconnection, * @param[in] ufrag - the ufrag * @param[in] pwd - the pwd * - * @return 0 success, error failure + * @return 0 for success; VCM_ERROR for failure */ static short vcmSetIceSessionParams_m(const char *peerconnection, char *ufrag, @@ -747,7 +758,7 @@ static short vcmStartIceChecks_m(const char *peerconnection, cc_boolean isContro { CSFLogDebug( logTag, "%s: PC = %s", __FUNCTION__, peerconnection); - sipcc::PeerConnectionWrapper pc(peerconnection); + sipcc::PeerConnectionWrapper pc(peerconnection); ENSURE_PC(pc, VCM_ERROR); nsresult res; @@ -801,7 +812,7 @@ short vcmStartIceChecks(const char *peerconnection, cc_boolean isControlling) * @param[in] ufrag - the ufrag * @param[in] pwd - the pwd * @param[in] candidates - the candidates - * @param[i] candidate_ct - the number of candidates + * @param[in] candidate_ct - the number of candidates * @return 0 success, error failure */ static short vcmSetIceMediaParams_m(const char *peerconnection, @@ -852,7 +863,7 @@ static short vcmSetIceMediaParams_m(const char *peerconnection, * @param[in] ufrag - the ufrag * @param[in] pwd - the pwd * @param[in] candidates - the candidates - * @param[i] candidate_ct - the number of candidates + * @param[in] candidate_ct - the number of candidates * @return 0 success, error failure */ short vcmSetIceMediaParams(const char *peerconnection, @@ -898,6 +909,8 @@ static short vcmCreateRemoteStream_m( uint32_t hints = 0; nsresult res; + *pc_stream_id = -1; + CSFLogDebug( logTag, "%s", __FUNCTION__); sipcc::PeerConnectionWrapper pc(peerconnection); ENSURE_PC(pc, VCM_ERROR); @@ -988,6 +1001,10 @@ static short vcmGetDtlsIdentity_m(const char *peerconnection, size_t max_digest_alg_len, char *digestp, size_t max_digest_len) { + + digest_algp[0]='\0'; + digestp[0]='\0'; + sipcc::PeerConnectionWrapper pc(peerconnection); ENSURE_PC(pc, VCM_ERROR); @@ -1078,7 +1095,7 @@ short vcmSetDataChannelParameters(const char *peerconnection, cc_uint16_t stream * @param[in] port_requested - requested port. * @param[in] listen_ip - local IP for listening * @param[in] is_multicast - multicast stream? - * @param[in,out] port_allocated - allocated(reserved) port + * @param[out] port_allocated - allocated(reserved) port * * tbd need to see if we can deprecate this API * @@ -1098,6 +1115,7 @@ short vcmRxOpen(cc_mcapid_t mcap_id, char fname[] = "vcmRxOpen"; char dottedIP[20] = ""; + *port_allocated = -1; if(listen_ip) { csf_sprintf(dottedIP, sizeof(dottedIP), "%u.%u.%u.%u", @@ -1243,7 +1261,7 @@ int vcmRxStart(cc_mcapid_t mcap_id, * @param[in] stream_id - stream id of the given media type. * @param[in] level - the m-line index * @param[in] pc_stream_id - the media stream index (from PC.addStream()) - * @param[i]n pc_track_id - the track within the media stream + * @param[in] pc_track_id - the track within the media stream * @param[in] call_handle - call handle * @param[in] peerconnection - the peerconnection in use * @param[in] num_payloads - number of negotiated payloads @@ -1471,11 +1489,11 @@ int vcmRxStartICE(cc_mcapid_t mcap_id, * @param[in] stream_id - stream id of the given media type. * @param[in] call_handle - call identifier * - * @return None + * @return 0 for success; VCM_ERROR for failure * */ -void vcmRxClose(cc_mcapid_t mcap_id, +short vcmRxClose(cc_mcapid_t mcap_id, cc_groupid_t group_id, cc_streamid_t stream_id, cc_call_handle_t call_handle) @@ -1487,7 +1505,7 @@ void vcmRxClose(cc_mcapid_t mcap_id, if (call_handle == CC_NO_CALL_ID) { CSFLogDebugS( logTag, "No CALL ID"); /* no operation when no call ID */ - return; + return VCM_ERROR; } switch ( mcap_id ) { @@ -1504,6 +1522,7 @@ void vcmRxClose(cc_mcapid_t mcap_id, default: break; } + return 0; } /** @@ -2084,7 +2103,7 @@ int vcmTxStartICE(cc_mcapid_t mcap_id, attrs, &ret), NS_DISPATCH_SYNC); - + return ret; } @@ -2097,10 +2116,10 @@ int vcmTxStartICE(cc_mcapid_t mcap_id, * @param[in] stream_id - stream id of the given media type. * @param[in] call_handle - call identifier * - * @return void + * @return 0 for success; VCM_ERROR for failure */ -void vcmTxClose(cc_mcapid_t mcap_id, +short vcmTxClose(cc_mcapid_t mcap_id, cc_groupid_t group_id, cc_streamid_t stream_id, cc_call_handle_t call_handle) @@ -2111,7 +2130,7 @@ void vcmTxClose(cc_mcapid_t mcap_id, if (call_handle == CC_NO_CALL_ID) { /* no operation when no call ID */ - return; + return VCM_ERROR; } switch ( mcap_id ) @@ -2129,6 +2148,7 @@ void vcmTxClose(cc_mcapid_t mcap_id, default: break; } + return 0; } #if 0 @@ -2287,6 +2307,8 @@ int vcmGetRtpStats(cc_mcapid_t mcap_id, char *rx_stats, char *tx_stats) { + rx_stats[0] = '\0'; + tx_stats[0] = '\0'; return 0; } diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c b/media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c index 51467b7a9fb3..67c94215d309 100755 --- a/media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c +++ b/media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c @@ -2918,8 +2918,10 @@ fsmdef_ev_createoffer (sm_event_t *event) { msg->data.session.constraints = 0; } - vcmGetIceParams(dcb->peerconnection, &ufrag, &ice_pwd); - if (!ufrag || !ice_pwd) { + vcm_res = vcmGetIceParams(dcb->peerconnection, &ufrag, &ice_pwd); + if (vcm_res) { + FSM_DEBUG_SM(DEB_F_PREFIX"vcmGetIceParams returned an error\n", + DEB_F_PREFIX_ARGS(FSM, __FUNCTION__)); ui_create_offer(evCreateOfferError, line, call_id, dcb->caller_id.call_instance_id, strlib_empty()); return (fsmdef_release(fcb, cause, FALSE)); @@ -3025,8 +3027,10 @@ fsmdef_ev_createanswer (sm_event_t *event) { msg->data.session.constraints = 0; } - vcmGetIceParams(dcb->peerconnection, &ufrag, &ice_pwd); - if (!ufrag || !ice_pwd) { + vcm_res = vcmGetIceParams(dcb->peerconnection, &ufrag, &ice_pwd); + if (vcm_res) { + FSM_DEBUG_SM(DEB_F_PREFIX"vcmGetIceParams returned an error\n", + DEB_F_PREFIX_ARGS(FSM, __FUNCTION__)); ui_create_offer(evCreateAnswerError, line, call_id, dcb->caller_id.call_instance_id, strlib_empty()); return (fsmdef_release(fcb, cause, FALSE)); diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c b/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c index 6f79878d8068..baf527761db2 100755 --- a/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c +++ b/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c @@ -638,16 +638,16 @@ lsm_open_rx (lsm_lcb_t *lcb, cc_action_data_open_rcv_t *data, char **candidates; int candidate_ct; char *default_addr; + short status; - vcmRxAllocICE(media->cap_index, dcb->group_id, media->refid, + status = vcmRxAllocICE(media->cap_index, dcb->group_id, media->refid, lsm_get_ms_ui_call_handle(lcb->line, lcb->call_id, lcb->ui_id), dcb->peerconnection, media->level, &default_addr, &port_allocated, &candidates, &candidate_ct); - // Check that we got a valid address and port - if (default_addr && (strlen(default_addr) > 0) && (port_allocated != -1)) { + if (!status) { sstrncpy(dcb->ice_default_candidate_addr, default_addr, sizeof(dcb->ice_default_candidate_addr)); data->port = (uint16_t)port_allocated; @@ -659,8 +659,10 @@ lsm_open_rx (lsm_lcb_t *lcb, cc_action_data_open_rcv_t *data, } } - LSM_DEBUG(get_debug_string(LSM_DBG_INT1), lcb->call_id, lcb->line, fname, - "allocated port", port_allocated); + if (rc == CC_RC_SUCCESS) { + LSM_DEBUG(get_debug_string(LSM_DBG_INT1), lcb->call_id, lcb->line, fname, + "allocated port", port_allocated); + } return (rc); } diff --git a/media/webrtc/signaling/src/sipcc/include/vcm.h b/media/webrtc/signaling/src/sipcc/include/vcm.h index 62e9c11f7bdf..ef607559c9a1 100755 --- a/media/webrtc/signaling/src/sipcc/include/vcm.h +++ b/media/webrtc/signaling/src/sipcc/include/vcm.h @@ -404,7 +404,7 @@ short vcmTxOpen(cc_mcapid_t mcap_id, * @param[in] port_requested - port requested (if zero -> give any) * @param[out] port_allocated - port that was actually allocated. * - * @return void + * @return 0 for success; VCM_ERROR for failure * */ @@ -416,7 +416,7 @@ void vcmRxAllocPort(cc_mcapid_t mcap_id, int *port_allocated); -void vcmRxAllocICE(cc_mcapid_t mcap_id, +short vcmRxAllocICE(cc_mcapid_t mcap_id, cc_groupid_t group_id, cc_streamid_t stream_id, cc_call_handle_t call_handle, @@ -435,9 +435,9 @@ void vcmRxAllocICE(cc_mcapid_t mcap_id, * @param[out] ufragp - where to put the ufrag * @param[out] pwdp - where to put the pwd * - * @return void + * @return 0 for success; VCM_ERROR for failure */ -void vcmGetIceParams(const char *peerconnection, char **ufragp, char **pwdp); +short vcmGetIceParams(const char *peerconnection, char **ufragp, char **pwdp); /* Set remote ICE global parameters. * @@ -685,11 +685,11 @@ int vcmTxStart(cc_mcapid_t mcap_id, * @param[in] stream_id - stream id of the given media type. * @param[in] call_handle - call handle * - * @return None + * @return 0 for success; VCM_ERROR for failure * */ -void vcmRxClose(cc_mcapid_t mcap_id, +short vcmRxClose(cc_mcapid_t mcap_id, cc_groupid_t group_id, cc_streamid_t stream_id, cc_call_handle_t call_handle); @@ -702,10 +702,10 @@ void vcmRxClose(cc_mcapid_t mcap_id, * @param[in] stream_id - stream id of the given media type. * @param[in] call_handle - call handle * - * @return void + * @return 0 for success; VCM_ERROR for failure */ -void vcmTxClose(cc_mcapid_t mcap_id, +short vcmTxClose(cc_mcapid_t mcap_id, cc_groupid_t group_id, cc_streamid_t stream_id, cc_call_handle_t call_handle);