Open
Bug 1023619
Opened 10 years ago
Updated 2 years ago
nr_stun_client_process_response needs some refactoring
Categories
(Core :: WebRTC: Networking, defect, P5)
Core
WebRTC: Networking
Tracking
()
NEW
backlog | tech-debt |
People
(Reporter: bwc, Unassigned)
Details
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.
Updated•9 years ago
|
backlog: --- → tech-debt
Rank: 45
Priority: -- → P4
Comment 1•7 years ago
|
||
Mass change P4->P5 to align with new Mozilla triage process.
Priority: P4 → P5
Updated•2 years ago
|
Severity: minor → S4
You need to log in
before you can comment on or make changes to this bug.
Description
•