[M] Change in libosmocore[master]: GSMTAP: allow configuring src IP for log messages

Attention is currently required from: osmith, laforge, pespin, msuraev. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31513 ) Change subject: GSMTAP: allow configuring src IP for log messages ...................................................................... Patch Set 11: Code-Review-1 (6 comments) Patchset: PS11: CR-1 due to the memleak. File include/osmocom/core/logging.h: https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/dce9223e_6518e0ad PS11, Line 414: log_set_src_ip This is the GSMTAP target specific API, so I believe it should be reflected in the symbol name, e.g. `log_set_gsmtap_src_ip`. https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/0a235fe2_e7611287 PS11, Line 444: log_target_create_gsmtap2 Now that you're adding `log_set[_gamtap]_src_ip()`, do we really need this new API? Wouldn't it be enough to call the old `log_target_create_gsmtap()` and then immediately `log_set[_gamtap]_src_ip()`? Ah, now I see that `log_set[_gamtap]_src_ip()` is using `gsmtap_source_init2()` internally. File src/core/logging_gsmtap.c: https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/62b526a5_1a76b538 PS11, Line 127: ip I think it can be a hostname, not necessarily an IP address? For instance, you can use 'localhost' and it should work. Not 100% sure though. https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/401bfc49_fbee37ef PS11, Line 135: gsmtap_source_init2 Looking at the code I can see `gsmtap_source_init2()` can handle `src == NULL`, so why not calling it unconditionally? https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/02d9c132_a77f97ea PS11, Line 145: target->tgt_gsmtap.src_addr What if `src_addr` was set before? You will be leaking memory here. Better use `osmo_talloc_replace_string()`. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31513 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9000269ce5b3dce1e757271b7c42e77b68d38f25 Gerrit-Change-Number: 31513 Gerrit-PatchSet: 11 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Wed, 08 Mar 2023 18:44:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment