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/.
Vadim Yanitskiy gerrit-no-reply at lists.osmocom.orgVadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/12402 ) Change subject: refactor log ctx for vlr_subscr and ran_conn ...................................................................... Patch Set 9: Code-Review-1 (12 comments) https://gerrit.osmocom.org/#/c/12402/9/include/osmocom/msc/ran_conn.h File include/osmocom/msc/ran_conn.h: https://gerrit.osmocom.org/#/c/12402/9/include/osmocom/msc/ran_conn.h@183 PS9, Line 183: const uint8_t mi[] Cosmetic: let's use 'const uint8_t *mi'. https://gerrit.osmocom.org/#/c/12402/9/src/libmsc/gsm_04_08.c File src/libmsc/gsm_04_08.c: https://gerrit.osmocom.org/#/c/12402/9/src/libmsc/gsm_04_08.c@304 PS9, Line 304: gsm48_mi_type_name(mi_type), mi_string This could be also changed to osmo_mi_name(...). https://gerrit.osmocom.org/#/c/12402/9/src/libmsc/gsm_04_08.c@752 PS9, Line 752: req->cm_service_type Looks like a mistake to me. The message says "mi_type ..." by you're refering CM Service type? I think it should be: ... "MI type %s is not expected\n", gsm48_mi_type_name(mi_type)... https://gerrit.osmocom.org/#/c/12402/9/src/libmsc/ran_conn.c File src/libmsc/ran_conn.c: https://gerrit.osmocom.org/#/c/12402/9/src/libmsc/ran_conn.c@696 PS9, Line 696: _ran_conn_update_id TBH, this looks unreadable. Why not to keep using a function? https://gerrit.osmocom.org/#/c/12402/9/src/libmsc/ran_conn.c@719 PS9, Line 719: const uint8_t mi[] same here https://gerrit.osmocom.org/#/c/12402/9/src/libmsc/ran_conn.c@719 PS9, Line 719: uint8_t and here https://gerrit.osmocom.org/#/c/12402/9/src/libvlr/vlr.c File src/libvlr/vlr.c: https://gerrit.osmocom.org/#/c/12402/9/src/libvlr/vlr.c@82 PS9, Line 82: vlr_subscr_name I couldn't resist and wrote an alternative version using the Pointer arithmetic. It looks good to me, but I guess only to me ;) If you like it, feel free to use: https://pastebin.com/jzaFwAeu https://gerrit.osmocom.org/#/c/12402/9/src/libvlr/vlr.c@89 PS9, Line 89: present You could just init buf[0] as '\0', and then check like this: buf[0] ? ":" : "" https://gerrit.osmocom.org/#/c/12402/9/src/libvlr/vlr.c@93 PS9, Line 93: snprintf What if this call would fail? https://gerrit.osmocom.org/#/c/12402/9/src/libvlr/vlr.c@101 PS9, Line 101: tmsi, sizeof(buf) Printing to tmsi, but sizeof(buf)? https://gerrit.osmocom.org/#/c/12402/9/src/libvlr/vlr.c@105 PS9, Line 105: snprintf(buf, sizeof(buf), Wouldn't this call overwrite the content of buffer? https://gerrit.osmocom.org/#/c/12402/9/src/libvlr/vlr_core.h File src/libvlr/vlr_core.h: https://gerrit.osmocom.org/#/c/12402/9/src/libvlr/vlr_core.h@7 PS9, Line 7: const This could be done in a separate change... -- To view, visit https://gerrit.osmocom.org/12402 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I66a68ce2eb8957a35855a3743d91a86299900834 Gerrit-Change-Number: 12402 Gerrit-PatchSet: 9 Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Max <msuraev at sysmocom.de> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com> Gerrit-Reviewer: dexter <pmaier at sysmocom.de> Gerrit-Comment-Date: Thu, 10 Jan 2019 14:05:30 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190110/01a5ee41/attachment.htm>