Patchwork gbproxy: Add gbprox_clear_patch_filter() (Coverity)

login
register
mail settings
Submitter Jacob Erlbeck
Date Aug. 14, 2014, 7:11 a.m.
Message ID <1408000264-9733-1-git-send-email-jerlbeck@sysmocom.de>
Download mbox | patch
Permalink /patch/379834/
State Accepted
Headers show

Comments

Jacob Erlbeck - Aug. 14, 2014, 7:11 a.m.
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(-)

Patch

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);