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.orgPatch Set 3: (4 comments) https://gerrit.osmocom.org/#/c/3537/3/src/osmux.c File src/osmux.c: Line 849: #define SNPRINTF_BUFFER_SIZE(ret, buffer_offset, size) \ > agreed; yet when first reading it, it helps to have doc for these non-obvio Agree, I'll add a short description of each parameter. Line 903: if (msg->len - msg_offset < sizeof(struct osmux_hdr)) { > ok, but let's have a unit test Agree Line 911: if (osmuxh->ft == OSMUX_FT_VOICE_AMR && !osmo_amr_ft_valid(osmuxh->amr_ft)) { > it is unrelated to range checking of snprintf calls. Should be separate IMH OK I'll split it Line 918: SNPRINTF_BUFFER_SIZE(ret, buf_offset, size); > There was an error log statement ("No room for OSMUX payload") that you're That's not checking the string output buffer, but the packet input buffer, it's different stuff. That check is there to prevent osmux_snprintf_payload() reading from put of bytes input buffer, and as our output buffer is full, there's no point in doing that check / print that error because anyway we are not interested in continuing and calling osmux_snprintf_payload(). So the early return imho is correct. -- To view, visit https://gerrit.osmocom.org/3537 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I695771d099833842db37a415b636035d17f1bba7 Gerrit-PatchSet: 3 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Holger Freyther <holger at freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pablo at gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de> Gerrit-HasComments: Yes