Patch Set 3: Code-Review-1 (5 comments)
https://gerrit.osmocom.org/#/c/2830/3/include/osmocom/gsm/gsm0808_utils.h File include/osmocom/gsm/gsm0808_utils.h: Line 77: * representation we use in struct gsm0808_speech_codec */ Typically we don't place comments with the function declarations in .h files. Not sure why this file has comments for each. The API doc belongs in the .c file as proper doxygen comments, and shouldn't be duplicated in the .h file. https://gerrit.osmocom.org/#/c/2830/3/src/gsm/gsm0808_utils.c File src/gsm/gsm0808_utils.c: Line 659: * representation we use in struct gsm0808_speech_codec */ Make this a proper doxygen comment, you will see that the other functions have them now when you rebase this patch onto current master. Line 665: * (See also 3GPP TS 48.008, 3.2.2.11 and 3.2.2.103) */ Does this comment rather belong joined to above API doc comment? Line 670: break; after a 'return' no 'break' necessary. Drop those. Line 694: break; in another patch you had a default: OSMO_ASSERT(false) ... why not here? Are we checking the -EINVAL return value at the caller? -- To view, visit https://gerrit.osmocom.org/2830 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib26a9c20864459b2baaa04f49b6e7902ba44b7cb Gerrit-PatchSet: 3 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-HasComments: Yes
