From 467cfda321f4d8cf1915f290aa6a89104dc28966 Mon Sep 17 00:00:00 2001 From: Daniel Adam Date: Fri, 22 Jul 2022 19:20:47 +0200 Subject: [PATCH] fixup! Fix issues reported by clang-tidy --- api/oc_buffer.c | 6 ++-- api/oc_helpers.c | 7 +++-- api/oc_ri.c | 4 +-- messaging/coap/coap.c | 5 ++- messaging/coap/engine.c | 29 +++++++++--------- messaging/coap/engine.h | 5 +-- messaging/coap/oscore.c | 61 +++++++++++++++++++++++++------------ security/oc_oscore_engine.c | 5 +-- security/oc_tls.c | 4 +-- 9 files changed, 78 insertions(+), 48 deletions(-) diff --git a/api/oc_buffer.c b/api/oc_buffer.c index cecf860a4b..d59535670c 100644 --- a/api/oc_buffer.c +++ b/api/oc_buffer.c @@ -188,16 +188,16 @@ OC_PROCESS_THREAD(message_buffer_handler, ev, data) data); } else { OC_DBG("Inbound network event: decrypted request"); - oc_process_post(&coap_engine, oc_events[INBOUND_RI_EVENT], data); + oc_process_post(&g_coap_engine, oc_events[INBOUND_RI_EVENT], data); } #else /* OC_OSCORE */ OC_DBG("Inbound network event: decrypted request"); - oc_process_post(&coap_engine, oc_events[INBOUND_RI_EVENT], data); + oc_process_post(&g_coap_engine, oc_events[INBOUND_RI_EVENT], data); #endif /* OC_OSCORE */ } #else /* OC_SECURITY */ OC_DBG("Inbound network event: decrypted request"); - oc_process_post(&coap_engine, oc_events[INBOUND_RI_EVENT], data); + oc_process_post(&g_coap_engine, oc_events[INBOUND_RI_EVENT], data); #endif /* !OC_SECURITY */ } else if (ev == oc_events[OUTBOUND_NETWORK_EVENT]) { oc_message_t *message = (oc_message_t *)data; diff --git a/api/oc_helpers.c b/api/oc_helpers.c index aef96b33a9..e1cce6c93e 100644 --- a/api/oc_helpers.c +++ b/api/oc_helpers.c @@ -17,7 +17,6 @@ #include "oc_helpers.h" #include "port/oc_assert.h" #include "port/oc_log.h" -#include #include static bool mmem_initialized = false; @@ -289,8 +288,10 @@ oc_conv_hex_string_to_byte_array(const char *hex_str, size_t hex_str_len, return -1; } - long len = lround((double)hex_str_len / 2.0); - + long len = hex_str_len / 2; + if ((hex_str_len % 2) != 0) { + ++len; + } if (len < 0 || *array_len < (size_t)len) { return -1; } diff --git a/api/oc_ri.c b/api/oc_ri.c index 299de48d39..afea0147a3 100644 --- a/api/oc_ri.c +++ b/api/oc_ri.c @@ -363,7 +363,7 @@ start_processes(void) allocate_events(); oc_process_start(&oc_etimer_process, NULL); oc_process_start(&g_timed_callback_events, NULL); - oc_process_start(&coap_engine, NULL); + oc_process_start(&g_coap_engine, NULL); oc_process_start(&message_buffer_handler, NULL); #ifdef OC_SECURITY @@ -388,7 +388,7 @@ stop_processes(void) oc_process_exit(&oc_network_events); oc_process_exit(&oc_etimer_process); oc_process_exit(&g_timed_callback_events); - oc_process_exit(&coap_engine); + oc_process_exit(&g_coap_engine); #ifdef OC_SECURITY #ifdef OC_OSCORE diff --git a/messaging/coap/coap.c b/messaging/coap/coap.c index d05e2a6655..e69659dadb 100644 --- a/messaging/coap/coap.c +++ b/messaging/coap/coap.c @@ -671,7 +671,10 @@ coap_oscore_parse_options(void *packet, uint8_t *data, uint32_t data_len, if (!outer || !oscore) { return BAD_OPTION_4_02; } - coap_parse_oscore_option(coap_pkt, current_option, option_length); + if (coap_parse_oscore_option(coap_pkt, current_option, option_length) != + 0) { + return BAD_OPTION_4_02; + } break; #endif /* OC_OSCORE && OC_SECURITY */ case COAP_OPTION_CONTENT_FORMAT: diff --git a/messaging/coap/engine.c b/messaging/coap/engine.c index 892d8137d3..3f5a346bae 100644 --- a/messaging/coap/engine.c +++ b/messaging/coap/engine.c @@ -46,9 +46,6 @@ */ #include "engine.h" -#include -#include -#include #include "api/oc_events.h" #include "api/oc_main.h" @@ -72,7 +69,11 @@ #include "coap_signal.h" #endif -OC_PROCESS(coap_engine, "CoAP Engine"); +#include +#include +#include + +OC_PROCESS(g_coap_engine, "CoAP Engine"); #ifdef OC_BLOCK_WISE extern bool oc_ri_invoke_coap_entity_handler( @@ -92,16 +93,16 @@ extern bool oc_ri_invoke_coap_entity_handler(void *request, void *response, // match is found, the message is dropped as it must be // a duplicate. #define OC_REQUEST_HISTORY_SIZE (25) -static uint16_t history[OC_REQUEST_HISTORY_SIZE]; -static uint8_t history_dev[OC_REQUEST_HISTORY_SIZE]; -static uint8_t idx; +static uint16_t g_history[OC_REQUEST_HISTORY_SIZE]; +static uint32_t g_history_dev[OC_REQUEST_HISTORY_SIZE]; +static uint8_t g_idx = 0; bool -oc_coap_check_if_duplicate(uint16_t mid, uint8_t device) +oc_coap_check_if_duplicate(uint16_t mid, uint32_t device) { size_t i; for (i = 0; i < OC_REQUEST_HISTORY_SIZE; i++) { - if (history[i] == mid && history_dev[i] == device) { + if (g_history[i] == mid && g_history_dev[i] == device) { OC_DBG("dropping duplicate request"); return true; } @@ -311,12 +312,12 @@ coap_receive(oc_message_t *msg) } else { #ifdef OC_REQUEST_HISTORY if (oc_coap_check_if_duplicate(message->mid, - (uint8_t)msg->endpoint.device)) { + (uint32_t)msg->endpoint.device)) { return 0; } - history[idx] = message->mid; - history_dev[idx] = (uint8_t)msg->endpoint.device; - idx = (idx + 1) % OC_REQUEST_HISTORY_SIZE; + g_history[g_idx] = message->mid; + g_history_dev[g_idx] = (uint32_t)msg->endpoint.device; + g_idx = (g_idx + 1) % OC_REQUEST_HISTORY_SIZE; #endif /* OC_REQUEST_HISTORY */ if (href_len == 7 && memcmp(href, "oic/res", 7) == 0) { coap_udp_init_message(response, COAP_TYPE_CON, CONTENT_2_05, @@ -948,7 +949,7 @@ coap_init_engine(void) coap_register_as_transaction_handler(); } /*---------------------------------------------------------------------------*/ -OC_PROCESS_THREAD(coap_engine, ev, data) +OC_PROCESS_THREAD(g_coap_engine, ev, data) { OC_PROCESS_BEGIN(); diff --git a/messaging/coap/engine.h b/messaging/coap/engine.h index 4ce195f64a..6f9609d7bf 100644 --- a/messaging/coap/engine.h +++ b/messaging/coap/engine.h @@ -52,17 +52,18 @@ #include "observe.h" #include "separate.h" #include "transactions.h" +#include #ifdef __cplusplus extern "C" { #endif -OC_PROCESS_NAME(coap_engine); +OC_PROCESS_NAME(g_coap_engine); void coap_init_engine(void); /*---------------------------------------------------------------------------*/ int coap_receive(oc_message_t *message); -bool oc_coap_check_if_duplicate(uint16_t mid, uint8_t device); +bool oc_coap_check_if_duplicate(uint16_t mid, uint32_t device); #ifdef __cplusplus } diff --git a/messaging/coap/oscore.c b/messaging/coap/oscore.c index 57560b1aea..9a77430379 100644 --- a/messaging/coap/oscore.c +++ b/messaging/coap/oscore.c @@ -21,6 +21,7 @@ #include "coap.h" #include "coap_signal.h" #include "oc_ri.h" +#include void oscore_send_error(const coap_packet_t *packet, uint8_t code, @@ -234,49 +235,71 @@ coap_parse_oscore_option(coap_packet_t *packet, const uint8_t *current_option, return 0; } /* Flags |0 0 0|h|k| n | */ - packet->oscore_flags = *current_option; + uint8_t oscore_flags = *current_option; current_option++; option_length--; - OC_DBG("\tflags: %02x", packet->oscore_flags); + OC_DBG("\tflags: %02x", oscore_flags); /* Partial IV length (n bytes) */ - packet->piv_len = (packet->oscore_flags & OSCORE_FLAGS_PIVLEN_BITMASK); - - if (packet->piv_len > 0) { + uint8_t piv[OSCORE_PIV_LEN]; + uint8_t piv_len = (oscore_flags & OSCORE_FLAGS_PIVLEN_BITMASK); + if (piv_len > 0) { /* Partial IV */ - memcpy(packet->piv, current_option, packet->piv_len); - current_option += packet->piv_len; - option_length -= packet->piv_len; + memcpy(piv, current_option, piv_len); + current_option += piv_len; + option_length -= piv_len; OC_DBG("\tPartial IV:"); - OC_LOGbytes(packet->piv, packet->piv_len); + OC_LOGbytes(piv, piv_len); } /* kid context (if any) */ /* Check if 'h' flag bit is set */ - if (packet->oscore_flags & OSCORE_FLAGS_KIDCTX_BITMASK) { - packet->kid_ctx_len = *current_option; + uint8_t kid_ctx[OSCORE_IDCTX_LEN]; + uint8_t kid_ctx_len = 0; + if ((oscore_flags & OSCORE_FLAGS_KIDCTX_BITMASK) != 0) { + kid_ctx_len = *current_option; current_option++; option_length--; /* Store kid context */ - memcpy(packet->kid_ctx, current_option, packet->kid_ctx_len); - current_option += packet->kid_ctx_len; - option_length -= packet->kid_ctx_len; + memcpy(kid_ctx, current_option, kid_ctx_len); + current_option += kid_ctx_len; + option_length -= kid_ctx_len; OC_DBG("\tkid context:"); - OC_LOGbytes(packet->kid_ctx, packet->kid_ctx_len); + OC_LOGbytes(kid_ctx, kid_ctx_len); } /* kid (if any) */ /* Check if 'k' flag bit is set */ - if (packet->oscore_flags & OSCORE_FLAGS_KID_BITMASK) { + uint8_t kid[OSCORE_CTXID_LEN]; + uint8_t kid_len = 0; + if ((oscore_flags & OSCORE_FLAGS_KID_BITMASK) != 0) { + if (option_length > UINT8_MAX) { + OC_ERR("oscore: invalid option length for kid(%zu)", option_length); + return -1; + } /* Remaining bytes in option: kid */ - packet->kid_len = option_length; - memcpy(packet->kid, current_option, option_length); + kid_len = (uint8_t)option_length; + memcpy(kid, current_option, option_length); OC_DBG("\tkid:"); - OC_LOGbytes(packet->kid, packet->kid_len); + OC_LOGbytes(kid, kid_len); + } + + packet->oscore_flags = oscore_flags; + packet->piv_len = piv_len; + if (piv_len > 0) { + memcpy(packet->piv, piv, piv_len); + } + packet->kid_ctx_len = kid_ctx_len; + if (kid_ctx_len > 0) { + memcpy(packet->kid_ctx, kid_ctx, kid_ctx_len); + } + packet->kid_len = kid_len; + if (kid_len > 0) { + memcpy(packet->kid, kid, kid_len); } return 0; diff --git a/security/oc_oscore_engine.c b/security/oc_oscore_engine.c index 2b67f9b5f0..9bbac7dfdb 100644 --- a/security/oc_oscore_engine.c +++ b/security/oc_oscore_engine.c @@ -120,7 +120,8 @@ oscore_parse_message(oc_message_t *message) if (oscore_pkt.transport_type == COAP_TRANSPORT_UDP && oscore_pkt.code <= OC_FETCH && - oc_coap_check_if_duplicate(oscore_pkt.mid, message->endpoint.device)) { + oc_coap_check_if_duplicate(oscore_pkt.mid, + (uint32_t)message->endpoint.device)) { OC_DBG("dropping duplicate request"); return false; } @@ -331,7 +332,7 @@ oc_oscore_recv_message(oc_message_t *message) OC_DBG("#################################"); /* Dispatch oc_message_t to the CoAP layer */ - if (oc_process_post(&coap_engine, oc_events[INBOUND_RI_EVENT], message) == + if (oc_process_post(&g_coap_engine, oc_events[INBOUND_RI_EVENT], message) == OC_PROCESS_ERR_FULL) { goto oscore_recv_error; } diff --git a/security/oc_tls.c b/security/oc_tls.c index 3413d2d638..c0b4a92430 100644 --- a/security/oc_tls.c +++ b/security/oc_tls.c @@ -2279,7 +2279,7 @@ read_application_data_tcp(oc_tls_peer_t *peer) (int)(total_length)); peer->processed_recv_message->encrypted = 0; memcpy(peer->processed_recv_message->endpoint.di.id, peer->uuid.id, 16); - if (oc_process_post(&coap_engine, oc_events[INBOUND_RI_EVENT], + if (oc_process_post(&g_coap_engine, oc_events[INBOUND_RI_EVENT], peer->processed_recv_message) == OC_PROCESS_ERR_FULL) { oc_message_unref(peer->processed_recv_message); @@ -2437,7 +2437,7 @@ read_application_data(oc_tls_peer_t *peer) #endif /* !OC_INOUT_BUFFER_SIZE */ } #else /* OC_OSCORE */ - if (oc_process_post(&coap_engine, oc_events[INBOUND_RI_EVENT], msg) == + if (oc_process_post(&g_coap_engine, oc_events[INBOUND_RI_EVENT], msg) == OC_PROCESS_ERR_FULL) { #ifndef OC_INOUT_BUFFER_SIZE oc_message_unref(msg);