Attention is currently required from: msuraev.

Patch set 1:Code-Review -2

View Change

1 comment:

• Commit Message:

  • Patch Set #1, Line 9: always return positive integer

    ... writing code against some potentially-could-be API which we do not even have plans introducing instead of writing it against current documented API is meaningless.

    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...

    ... especially given that other uses of such functions like osmo_gsm48_rest_octets_si1_encode() do not have this kind of useless checks.

    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 change 31091. To unsubscribe, or for help writing mail filters, visit settings.