Attention is currently required from: msuraev. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31091 ) Change subject: SI13: drop meaningless error check ...................................................................... Patch Set 1: Code-Review-2 (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-bsc/+/31091/comment/21d871b0_8184acdb PS1, Line 9: always return positive integer Signed rc is already the API, so a negative value is possible from the API point of view. There is nothing in the docstring saying that the returned value is always positive. You're simply assuming that the implementation of osmo_gsm48_rest_octets_si13_encode(), which is a library function, will never change. How can you be sure about that... Actually osmo_gsm48_rest_octets_si2quater_encode() may return a negative value, and there is such a 'useless' check in make_si2quaters(). Also, the unit test for osmo_gsm48_rest_octets_si13_encode() in libosmocore.git does check if rc is negative. These checks are not useless after all, and it's sad that we don't have them in all cases. So IMO this patch is meaningless. I am against merging it. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31091 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ib1aeaa57ad0929fd2e1296737969d89485f0ac7b Gerrit-Change-Number: 31091 Gerrit-PatchSet: 1 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Sat, 04 Feb 2023 20:09:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment