Bug 1644477: Make candidate pair insertion code easier to read/understand. r=mjf
authorByron Campen [:bwc] <docfaraday@gmail.com>
Tue, 09 Jun 2020 19:44:50 +0000
changeset 598943 45c61fd0206e7505f4d950a876279ba30a203321
parent 598942 3a6ed2262ba4a316645d1b9450f1fa7631faba19
child 598944 944303739df55f2c6014811ef44d23246d04aa8d
push id13310
push userffxbld-merge
push dateMon, 29 Jun 2020 14:50:06 +0000
treeherdermozilla-beta@15a59a0afa5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmjf
bugs1644477
milestone79.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1644477: Make candidate pair insertion code easier to read/understand. r=mjf Includes removing an error code for a function that never fails, and removing an error return when the function successfully did what it said it would. Differential Revision: https://phabricator.services.mozilla.com/D78929
media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.h
media/mtransport/third_party/nICEr/src/ice/ice_component.c
--- a/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
@@ -420,18 +420,17 @@ static int nr_ice_candidate_copy_for_tri
     if(r=nr_ice_candidate_pair_create(pair->pctx, pair->local, pair->remote, &copy))
       ABORT(r);
 
     /* Preserve nomination status */
     copy->peer_nominated= pair->peer_nominated;
     copy->nominated = pair->nominated;
 
     r_log(LOG_ICE,LOG_INFO,"CAND-PAIR(%s): Adding pair to check list and trigger check queue: %s",pair->codeword,pair->as_string);
-    if(r=nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,copy))
-      ABORT(r);
+    nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,copy);
     nr_ice_candidate_pair_trigger_check_append(&pair->remote->stream->trigger_check_queue,copy);
 
     copy->triggered = 1;
     nr_ice_candidate_pair_set_state(copy->pctx,copy,NR_ICE_PAIR_STATE_WAITING);
 
     _status=0;
   abort:
     return(_status);
@@ -598,32 +597,30 @@ int nr_ice_candidate_pair_trigger_check_
        pair->triggered_check_queue_entry.tqe_prev)
       return(0);
 
     TAILQ_INSERT_TAIL(head,pair,triggered_check_queue_entry);
 
     return(0);
   }
 
-int nr_ice_candidate_pair_insert(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair)
+void nr_ice_candidate_pair_insert(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair)
   {
     nr_ice_cand_pair *c1;
 
     c1=TAILQ_FIRST(head);
     while(c1){
       if(c1->priority < pair->priority){
         TAILQ_INSERT_BEFORE(c1,pair,check_queue_entry);
         break;
       }
 
       c1=TAILQ_NEXT(c1,check_queue_entry);
     }
     if(!c1) TAILQ_INSERT_TAIL(head,pair,check_queue_entry);
-
-    return(0);
   }
 
 void nr_ice_candidate_pair_restart_stun_nominated_cb(NR_SOCKET s, int how, void *cb_arg)
   {
     nr_ice_cand_pair *pair=cb_arg;
     int r,_status;
 
     pair->restart_nominated_cb_timer=0;
--- a/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.h
@@ -83,17 +83,17 @@ struct nr_ice_cand_pair_ {
 int nr_ice_candidate_pair_create(nr_ice_peer_ctx *pctx, nr_ice_candidate *lcand,nr_ice_candidate *rcand,nr_ice_cand_pair **pairp);
 int nr_ice_candidate_pair_unfreeze(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair);
 int nr_ice_candidate_pair_start(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair);
 void nr_ice_candidate_pair_set_state(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair, int state);
 void nr_ice_candidate_pair_dump_state(nr_ice_cand_pair *pair, int log_level);
 void nr_ice_candidate_pair_cancel(nr_ice_peer_ctx *pctx,nr_ice_cand_pair *pair, int move_to_wait_state);
 int nr_ice_candidate_pair_select(nr_ice_cand_pair *pair);
 int nr_ice_candidate_pair_do_triggered_check(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair);
-int nr_ice_candidate_pair_insert(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair);
+void nr_ice_candidate_pair_insert(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair);
 int nr_ice_candidate_pair_trigger_check_append(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair);
 void nr_ice_candidate_pair_restart_stun_nominated_cb(NR_SOCKET s, int how, void *cb_arg);
 int nr_ice_candidate_pair_destroy(nr_ice_cand_pair **pairp);
 void nr_ice_candidate_pair_role_change(nr_ice_cand_pair *pair);
 
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
--- a/media/mtransport/third_party/nICEr/src/ice/ice_component.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_component.c
@@ -1679,48 +1679,44 @@ int nr_ice_component_finalize(nr_ice_com
     }
 
     return(0);
   }
 
 
 int nr_ice_component_insert_pair(nr_ice_component *pcomp, nr_ice_cand_pair *pair)
   {
-    int r,_status;
-    int pair_inserted=0;
+    int _status;
 
     /* Pairs for peer reflexive are marked SUCCEEDED immediately */
     if (pair->state != NR_ICE_PAIR_STATE_FROZEN &&
         pair->state != NR_ICE_PAIR_STATE_SUCCEEDED){
       assert(0);
       ABORT(R_BAD_ARGS);
     }
 
-    if(r=nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,pair))
-      ABORT(r);
-
-    pair_inserted=1;
+    /* We do not throw an error after this, because we've inserted the pair. */
+    nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,pair);
 
     /* Make sure the check timer is running, if the stream was previously
      * started. We will not start streams just because a pair was created,
      * unless it is the first pair to be created across all streams. */
     r_log(LOG_ICE,LOG_DEBUG,"ICE-PEER(%s)/CAND-PAIR(%s): Ensure that check timer is running for new pair %s.",pair->remote->stream->pctx->label, pair->codeword, pair->as_string);
 
     if(pair->remote->stream->ice_state == NR_ICE_MEDIA_STREAM_CHECKS_ACTIVE ||
        (pair->remote->stream->ice_state == NR_ICE_MEDIA_STREAM_CHECKS_FROZEN &&
         !pair->remote->stream->pctx->checks_started)){
       if(nr_ice_media_stream_start_checks(pair->remote->stream->pctx, pair->remote->stream)) {
         r_log(LOG_ICE,LOG_WARNING,"ICE-PEER(%s)/CAND-PAIR(%s): Could not restart checks for new pair %s.",pair->remote->stream->pctx->label, pair->codeword, pair->as_string);
-        ABORT(R_INTERNAL);
       }
     }
 
     _status=0;
   abort:
-    if (_status && !pair_inserted) {
+    if (_status) {
       nr_ice_candidate_pair_destroy(&pair);
     }
     return(_status);
   }
 
 int nr_ice_component_get_default_candidate(nr_ice_component *comp, nr_ice_candidate **candp, int ip_version)
   {
     int _status;