Open Bug 1023619 Opened 6 years ago Updated 2 years ago
_stun _client _process _response needs some refactoring
There are a few things that bug me about nr_stun_client_process_response: - One large function that handles two distinct duties; determining whether a STUN response's transaction id matches that of the stun context, and performing the processing of that response if so. Writing an abort: section that makes sense for both of these cases is a bit fiddly, particularly wrt logging errors. - This function overwrites ctx->response even when the transaction id does not match, which is probably asking for confusion. - This function decodes the STUN response, meaning that we decode the same response multiple times while searching for a matching context. This is a bit silly, although probably not that harmful. - It is unclear to me whether we even need a |response| field in nr_stun_client_ctx; the only places that use it outside of this function are only interested in the error code, which is recorded in another field already.
Mass change P4->P5 to align with new Mozilla triage process.
Priority: P4 → P5
You need to log in before you can comment on or make changes to this bug.