Change in osmo-msc[master]: SMPP: move read/write callbacks to libsmpputil

Attention is currently required from: osmith, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28848 ) Change subject: SMPP: move read/write callbacks to libsmpputil ...................................................................... Patch Set 28: (6 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/0f4c540b_64b8ea6a PS28, Line 7: SMPP: move read/write callbacks to libsmpputil would be good to give a reason for the move / which other caller is currently expected File include/osmocom/smpp/smpp.h: https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/dcbf9993_b50aaa3a PS28, Line 51: + * osmo_fd to properly check for errors. you have diff '+' leaked into the C code https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/683ef905_fe48acca PS28, Line 53: + * an error. The code there should handle closing the connection. I would prefer to keep this macro confined locally to a .c file, i guess smpp_utils.c - contains flow control (not local to a .c file but in a header file) - requiring a label - requiring a specific return type in calling code If this makes it into public API, it needs a more detailed explanation, particularly about the 'return 0' meaning "try again". I guess best would be a code example of what the macro expects. https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/95e06b96_0280d326 PS28, Line 55: + * projects as well. hmm idk. this should have a name that is different from the general libosmocore-style to avoid naming conflicts. For example if you plan to move it to libosmocore, then you can clearly switch from one to the other. like SMPP_CHECK_FD_READ File src/libsmpputil/smpp_utils.c: https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/75c3dae7_8579125f PS28, Line 58: /* !\brief call-back when per-ESME TCP socket has some data to be read */ drop \brief, see https://osmocom.org/projects/cellular-infrastructure/wiki/Guidelines_for_AP… https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/8448af0d_bece25b5 PS28, Line 117: int esme_write_callback(struct esme *esme, int fd, struct msgb *msg) documentation for args and rc? (I tried to understand the 'exit(99)' below and found no explanation for rc == 0) -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28848 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I875eb5249004d3a960aee46c5099592d18fcaa76 Gerrit-Change-Number: 28848 Gerrit-PatchSet: 28 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Thu, 15 Sep 2022 11:43:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment