Attention is currently required from: neels, msuraev.
Patch set 8:Code-Review -1
8 comments:
Commit Message:
Patch Set #8, Line 9: Limit length of optional Data parameter is 130 bytes according to ITU-T Rec Q.713 §4.2..§4.5. If we receive CR, CC or
This reads a bit weird. I suggest to reword this sentence a bit: "The length limit of optional Data parameter is 130 bytes according to ...".
File src/sccp_scoc.c:
Patch Set #8, Line 50: #include <osmocom/core/msgb.h>
cosmetic: move it below to other imports from 'osmocom/core'.
Patch Set #8, Line 603: xua_class_msg_name
xua_class_msg_name() operates on a static buffer, so calling it more than once in a logging statement may result in printing the same value twice.
IIUC, this value is always SCCP_MAX_DATA. Do we really need to accept it as an argument?
Patch Set #8, Line 623: xua_opt_data_cache_check
When I see function names ending with _check, I expect them to do nothing else but check something and return some outcome. But here _check actually means populate the cache if necessary and return true if caching is not needed. This is confusing. I suggest to rename it to xua_opt_data_cache_populate() or so, and return true if the cache was populated.
Patch Set #8, Line 650: static bool xua_opt_data_length_check(struct sccp_connection *conn, const struct osmo_scu_prim *prim, int msg_type)
Again, this naming is really confusing. _opt_data_length_check implies that this function does check length of optional data and return an outcome. In reality this function not only does the length checks, but may also cache data or even Tx it by calling osmo_sccp_tx_data().
Patch Set #8, Line 659: break
unreachable
Patch Set #8, Line 1006: xua_opt_data_clear_cache
IMO, this should be done in conn_destroy().
To view, visit change 29084. To unsubscribe, or for help writing mail filters, visit settings.