Attention is currently required from: osmith, laforge, pespin, msuraev.
Patch set 11:Code-Review -1
6 comments:
Patchset:
CR-1 due to the memleak.
File include/osmocom/core/logging.h:
Patch Set #11, 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`.
Patch Set #11, 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:
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.
Patch Set #11, 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?
Patch Set #11, 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 change 31513. To unsubscribe, or for help writing mail filters, visit settings.