This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Pau Espin Pedrol gerrit-no-reply at lists.osmocom.orgPau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/12017 ) Change subject: msgb: add test helper ...................................................................... Patch Set 1: Code-Review-1 (7 comments) https://gerrit.osmocom.org/#/c/12017/1/include/osmocom/core/msgb.h File include/osmocom/core/msgb.h: https://gerrit.osmocom.org/#/c/12017/1/include/osmocom/core/msgb.h@569 PS1, Line 569: bool msgb_cmpr(const char *f, size_t l, const struct msgb *msg, const uint8_t *data, size_t len, bool print); strcmp, so why not msgb_cmp instead of msgb_cmpr? https://gerrit.osmocom.org/#/c/12017/1/src/msgb.c File src/msgb.c: https://gerrit.osmocom.org/#/c/12017/1/src/msgb.c@176 PS1, Line 176: static inline bool compare_n_print(const char *f, size_t l, uint8_t level, remove inline, let the compiler decide what's more optimal. https://gerrit.osmocom.org/#/c/12017/1/src/msgb.c@196 PS1, Line 196: /* N. B: that's intended to be used bymsgb_cmpr*() so length check is already passed, This comment needs to be moved to line 180 since len is already used there. https://gerrit.osmocom.org/#/c/12017/1/src/msgb.c@230 PS1, Line 230: /*! Compare and print: check data in msgb against given data and print erros if any typo: errors https://gerrit.osmocom.org/#/c/12017/1/src/msgb.c@231 PS1, Line 231: * \param[in] f text prefix, usually __func__, ignored if print == false If it's based on print value, then it makes sense to move it at the end after print, not here at the start of the function. BTW, you should move these functions to have an underscore before it (_msg_cmp) and then have a define as msg_cmp(foo, bar) _msg_cmp(foo, bar, __FILE__, __LINE__). We do that in fsm.h check it. https://gerrit.osmocom.org/#/c/12017/1/src/msgb.c@232 PS1, Line 232: * \param[in] l numeric prefix, usually __LINE__, ignored if print == false Same, move at the end. https://gerrit.osmocom.org/#/c/12017/1/src/msgb.c@239 PS1, Line 239: bool msgb_cmpr(const char *f, size_t l, const struct msgb *msg, const uint8_t *data, size_t len, bool print) I'd expect a msgb_cmp to compare two structs of msgb, so better calling this one msgb_cmp_data, in case we want to add the other one later. Now that we are adding the API, what about having it return an int like memcmp? after all we are using memcmp, so we can return an int. -- To view, visit https://gerrit.osmocom.org/12017 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3bc95f2f5ab6e3f4b502647fb3e0aaaf1f7c4cf5 Gerrit-Change-Number: 12017 Gerrit-PatchSet: 1 Gerrit-Owner: Max <msuraev at sysmocom.de> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de> Gerrit-Comment-Date: Fri, 30 Nov 2018 11:44:47 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181130/bc5715e0/attachment.htm>