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/.
Neels Hofmeyr gerrit-no-reply at lists.osmocom.orgPatch Set 2: Code-Review-1 (2 comments) One thing that's worse after this patch: we add a couple of short-lived small dynamic allocations (gsup message, msisdn, apn). The root cause being that we failed to use actual arrays in the original definition of the gsup message struct. Instead of talloc, we could pass in non-dynamic buffers (maybe even in a struct definition with the sole purpose of providing uint8_t[] for a gsup message), or we could use static buffers within the new function (and make it non-threadsafe, which is ok because we use select() to handle messages one after the other anyway). What do others think, is avoiding dynamic allocation micro-optimisation or worth rewriting this patch for? https://gerrit.osmocom.org/#/c/7992/2/src/gsup_server.c File src/gsup_server.c: Line 369: gsup = talloc_zero(NULL, struct osmo_gsup_message); do not talloc from NULL ctx, pass a ctx in as arg. If you talloc at NULL, we are likely to miss memory leaks should they show up. Line 373: osmo_strlcpy(gsup->imsi, imsi, GSM23003_IMSI_MAX_DIGITS + 1); wouldn't it make sense to use sizeof(gsup->imsi) instead of guessing the same constants again, hopefully correctly? -- To view, visit https://gerrit.osmocom.org/7992 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6a92ca34cdaadca9eacc774bb1ca386c325ba865 Gerrit-PatchSet: 2 Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de> Gerrit-HasComments: Yes