From 24648ad088c4feea39f8e9fda82c35d207af45e0 Mon Sep 17 00:00:00 2001 From: Juraj Sloboda Date: Tue, 6 Sep 2016 04:43:52 -0700 Subject: Fix bugs in IPFIX code discovered by coverity Change-Id: Ibe6ccb99c3c29c14efb34191f209a2f6a14293f7 Signed-off-by: Juraj Sloboda --- vnet/vnet/flow/flow_report.c | 20 ++++++++++++++++++++ vnet/vnet/flow/flow_report.h | 2 ++ vnet/vnet/flow/flow_report_classify.c | 31 +++++++------------------------ vpp-api-test/vat/api_format.c | 2 +- vpp/vpp-api/api.c | 21 +++++++++------------ 5 files changed, 39 insertions(+), 37 deletions(-) diff --git a/vnet/vnet/flow/flow_report.c b/vnet/vnet/flow/flow_report.c index 932613d338d..cbf1313d0e5 100644 --- a/vnet/vnet/flow/flow_report.c +++ b/vnet/vnet/flow/flow_report.c @@ -300,6 +300,26 @@ int vnet_flow_report_add_del (flow_report_main_t *frm, return 0; } +clib_error_t * flow_report_add_del_error_to_clib_error (int error) +{ + switch (error) + { + case 0: + return 0; + case VNET_API_ERROR_NO_SUCH_ENTRY: + return clib_error_return (0, "Flow report not found"); + case VNET_API_ERROR_VALUE_EXIST: + return clib_error_return (0, "Flow report already exists"); + case VNET_API_ERROR_INVALID_VALUE: + return clib_error_return (0, "Expecting either still unused values " + "for both domain_id and src_port " + "or already used values for both fields"); + default: + return clib_error_return (0, "vnet_flow_report_add_del returned %d", + error); + } +} + void vnet_flow_reports_reset (flow_report_main_t * frm) { flow_report_t *fr; diff --git a/vnet/vnet/flow/flow_report.h b/vnet/vnet/flow/flow_report.h index 7b9fc7b0fc7..4e764377dc8 100644 --- a/vnet/vnet/flow/flow_report.h +++ b/vnet/vnet/flow/flow_report.h @@ -132,6 +132,8 @@ typedef struct { int vnet_flow_report_add_del (flow_report_main_t *frm, vnet_flow_report_add_del_args_t *a); +clib_error_t * flow_report_add_del_error_to_clib_error (int error); + void vnet_flow_reports_reset (flow_report_main_t * frm); void vnet_stream_reset (flow_report_main_t * frm, u32 stream_index); diff --git a/vnet/vnet/flow/flow_report_classify.c b/vnet/vnet/flow/flow_report_classify.c index 95403793cfe..cb8fe069681 100644 --- a/vnet/vnet/flow/flow_report_classify.c +++ b/vnet/vnet/flow/flow_report_classify.c @@ -391,9 +391,10 @@ ipfix_classify_table_add_del_command_fn (vlib_main_t * vm, ipfix_classify_table_t * table; int rv; int is_add = -1; - u32 classify_table_index; + u32 classify_table_index = ~0; u8 ip_version = 0; u8 transport_protocol = 255; + clib_error_t * error = 0; if (fcm->src_port == 0) clib_error_return (0, "call 'set ipfix classify stream' first"); @@ -458,31 +459,13 @@ ipfix_classify_table_add_del_command_fn (vlib_main_t * vm, rv = vnet_flow_report_add_del (frm, &args); - switch (rv) - { - case 0: - break; - case VNET_API_ERROR_NO_SUCH_ENTRY: - return clib_error_return (0, "Flow report not found"); - case VNET_API_ERROR_VALUE_EXIST: - return clib_error_return (0, "Flow report already exists"); - case VNET_API_ERROR_INVALID_VALUE: - return clib_error_return (0, "Expecting either still unused values " - "for both domain_id and src_port " - "or already used values for both fields"); - default: - return clib_error_return (0, "vnet_flow_report_add_del returned %d", rv); - } + error = flow_report_add_del_error_to_clib_error(rv); - if (is_add) { - if (rv != 0) - ipfix_classify_delete_table(table - fcm->tables); - } else { - if (rv == 0) - ipfix_classify_delete_table(table - fcm->tables); - } + /* If deleting, or add failed */ + if (is_add == 0 || (rv && is_add)) + ipfix_classify_delete_table (table - fcm->tables); - return 0; + return error; } VLIB_CLI_COMMAND (ipfix_classify_table_add_del_command, static) = { diff --git a/vpp-api-test/vat/api_format.c b/vpp-api-test/vat/api_format.c index 128ba40aa96..0df2051e6c7 100644 --- a/vpp-api-test/vat/api_format.c +++ b/vpp-api-test/vat/api_format.c @@ -8733,7 +8733,7 @@ api_ipfix_classify_table_add_del (vat_main_t * vam) unformat_input_t *i = vam->input; vl_api_ipfix_classify_table_add_del_t *mp; int is_add = -1; - u32 classify_table_index; + u32 classify_table_index = ~0; u8 ip_version = 0; u8 transport_protocol = 255; f64 timeout; diff --git a/vpp/vpp-api/api.c b/vpp/vpp-api/api.c index 36532434df2..44afd6745f6 100644 --- a/vpp/vpp-api/api.c +++ b/vpp/vpp-api/api.c @@ -7894,6 +7894,8 @@ vl_api_classify_session_dump_t_handler (vl_api_classify_session_dump_t * mp) vnet_classify_table_t *t; q = vl_api_client_index_to_input_queue (mp->client_index); + if (!q) + return; /* *INDENT-OFF* */ pool_foreach (t, cm->tables, @@ -8094,6 +8096,8 @@ static void vl_api_ipfix_classify_stream_details_t *rmp; q = vl_api_client_index_to_input_queue (mp->client_index); + if (!q) + return; rmp = vl_msg_api_alloc (sizeof (*rmp)); memset (rmp, 0, sizeof (*rmp)); @@ -8174,19 +8178,10 @@ static void args.src_port = fcm->src_port; rv = vnet_flow_report_add_del (frm, &args); - if (rv != 0) - goto out; - if (is_add) - { - if (rv != 0) - ipfix_classify_delete_table (table - fcm->tables); - } - else - { - if (rv == 0) - ipfix_classify_delete_table (table - fcm->tables); - } + /* If deleting, or add failed */ + if (is_add == 0 || (rv && is_add)) + ipfix_classify_delete_table (table - fcm->tables); out: REPLY_MACRO (VL_API_SET_IPFIX_CLASSIFY_STREAM_REPLY); @@ -8222,6 +8217,8 @@ static void u32 i; q = vl_api_client_index_to_input_queue (mp->client_index); + if (!q) + return; for (i = 0; i < vec_len (fcm->tables); i++) if (ipfix_classify_table_index_valid (i)) -- cgit 1.2.3-korg