Add a separate function to clear the IMSI filter to be used instead of gbprox_set_patch_filter(cfg, NULL, ...). Albeit it fixes a Coverity issue (Unchecked return value), it is a false positive, since the return value is always 0 in these cases. Nevertheless it is more obvious what happens when an explicit clear function is called. Using NULL as filter argument of gbprox_set_patch_filter still clears the filter. Fixes: Coverity CID 1231255 Sponsored-by: On-Waves ehf --- openbsc/include/openbsc/gb_proxy.h | 1 + openbsc/src/gprs/gb_proxy.c | 13 +++++++++---- openbsc/src/gprs/gb_proxy_vty.c | 4 ++-- openbsc/tests/gbproxy/gbproxy_test.c | 6 ++++++ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/openbsc/include/openbsc/gb_proxy.h b/openbsc/include/openbsc/gb_proxy.h index 039a9a1..d6dde10 100644 --- a/openbsc/include/openbsc/gb_proxy.h +++ b/openbsc/include/openbsc/gb_proxy.h @@ -115,6 +115,7 @@ void gbprox_reset(struct gbproxy_config *cfg); int gbprox_set_patch_filter(struct gbproxy_config *cfg, const char *filter, const char **err_msg); +void gbprox_clear_patch_filter(struct gbproxy_config *cfg); void gbprox_delete_tlli(struct gbproxy_peer *peer, struct gbproxy_tlli_info *tlli_info); diff --git a/openbsc/src/gprs/gb_proxy.c b/openbsc/src/gprs/gb_proxy.c index a8ce3cc..64fb55b 100644 --- a/openbsc/src/gprs/gb_proxy.c +++ b/openbsc/src/gprs/gb_proxy.c @@ -481,16 +481,21 @@ static void gbprox_delete_tllis(struct gbproxy_peer *peer) OSMO_ASSERT(llist_empty(&state->enabled_tllis)); } +void gbprox_clear_patch_filter(struct gbproxy_config *cfg) +{ + if (cfg->check_imsi) { + regfree(&cfg->imsi_re_comp); + cfg->check_imsi = 0; + } +} + int gbprox_set_patch_filter(struct gbproxy_config *cfg, const char *filter, const char **err_msg) { static char err_buf[300]; int rc; - if (cfg->check_imsi) { - regfree(&cfg->imsi_re_comp); - cfg->check_imsi = 0; - } + gbprox_clear_patch_filter(cfg); if (!filter) return 0; diff --git a/openbsc/src/gprs/gb_proxy_vty.c b/openbsc/src/gprs/gb_proxy_vty.c index ec73ae6..9574a45 100644 --- a/openbsc/src/gprs/gb_proxy_vty.c +++ b/openbsc/src/gprs/gb_proxy_vty.c @@ -193,7 +193,7 @@ static int set_core_apn(struct vty *vty, const char *apn, const char *filter) talloc_free(g_cfg->core_apn); g_cfg->core_apn = NULL; g_cfg->core_apn_size = 0; - gbprox_set_patch_filter(g_cfg, NULL, NULL); + gbprox_clear_patch_filter(g_cfg); return CMD_SUCCESS; } @@ -206,7 +206,7 @@ static int set_core_apn(struct vty *vty, const char *apn, const char *filter) } if (!filter) { - gbprox_set_patch_filter(g_cfg, NULL, NULL); + gbprox_clear_patch_filter(g_cfg); } else if (gbprox_set_patch_filter(g_cfg, filter, &err_msg) != 0) { vty_out(vty, "Match expression invalid: %s%s", err_msg, VTY_NEWLINE); diff --git a/openbsc/tests/gbproxy/gbproxy_test.c b/openbsc/tests/gbproxy/gbproxy_test.c index 964c6da..5363749 100644 --- a/openbsc/tests/gbproxy/gbproxy_test.c +++ b/openbsc/tests/gbproxy/gbproxy_test.c @@ -1571,6 +1571,12 @@ static void test_gbproxy_imsi_matching(void) OSMO_ASSERT(gbprox_set_patch_filter(&cfg, NULL, &err_msg) == 0); OSMO_ASSERT(cfg.check_imsi == 0); + OSMO_ASSERT(gbprox_set_patch_filter(&cfg, filter_re2, &err_msg) == 0); + OSMO_ASSERT(cfg.check_imsi == 1); + + gbprox_clear_patch_filter(&cfg); + OSMO_ASSERT(cfg.check_imsi == 0); + peer = gbproxy_peer_alloc(&cfg, 20); OSMO_ASSERT(gbprox_set_patch_filter(&cfg, filter_re2, &err_msg) == 0); -- 1.9.1