From 4e125e3af7b9fecfa76d0960af1c7e47285ff22e Mon Sep 17 00:00:00 2001 From: Maximilian Nitsch Date: Wed, 20 May 2026 17:09:49 +0200 Subject: [PATCH] fix: cppcheck warnings Add command-line options for cppcheck to the CI job: - `--suppress=constParameterCallback` - `--check-level=exhaustive` information: Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches. - `--force` information: Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. Signed-off-by: Maximilian Nitsch --- .gitlab-ci.yml | 11 +++- components/aap2/aap2_agent.c | 6 ++ components/agents/echo_agent.c | 5 ++ components/agents/posix/application_agent.c | 6 ++ components/agents/storage/sqlite_agent.c | 2 +- components/cla/bibe_proto.c | 38 ++++++++---- components/cla/cla.c | 53 +++++++++++------ components/cla/posix/cla_bibe.c | 6 +- components/cla/posix/cla_tcp_common.c | 24 +++++--- components/cla/posix/cla_tcp_util.c | 5 ++ components/platform/posix/simple_queue.c | 6 ++ components/routing/compat/config_parser.c | 12 ++++ components/ud3tn/bundle.c | 5 +- components/ud3tn/bundle_processor.c | 2 +- components/ud3tn/eid.c | 65 ++++++++++----------- include/cla/bibe_proto.h | 2 +- include/ud3tn/bundle.h | 2 +- test/unit/test_bibe_encoder.c | 4 +- 18 files changed, 173 insertions(+), 81 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 35f19fa..13b72ed 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -787,14 +787,19 @@ cppcheck-static-analyzer: stage: code_quality_test when: always script: - - cppcheck --enable=warning --error-exitcode=2 --inline-suppr -I include -I external/tinycbor/src -I external/util/include components + - cppcheck --version + - cppcheck + --enable=warning --force --check-level=exhaustive --error-exitcode=2 --inline-suppr + -I include -I external/tinycbor/src -I external/util/include components cppcheck-stylecheck: stage: code_quality_test when: always script: - # NOTE: When updating cppcheck, add the following arguments: --check-level=exhaustive --suppress=constParameterCallback - - cppcheck --enable=style --error-exitcode=2 --inline-suppr --suppress=knownConditionTrueFalse + - cppcheck --version + - cppcheck + --enable=style --force --check-level=exhaustive --error-exitcode=2 --inline-suppr + --suppress=knownConditionTrueFalse --suppress=constParameterCallback -I include -I external/tinycbor/src -I external/util/include components publish-docker-image: diff --git a/components/aap2/aap2_agent.c b/components/aap2/aap2_agent.c index a1698b0..c9317c7 100644 --- a/components/aap2/aap2_agent.c +++ b/components/aap2/aap2_agent.c @@ -733,6 +733,12 @@ static aap2_ResponseStatus process_adu_msg( const size_t ar_size = payload_length + ar_typecode_size + 1; uint8_t *const ar_bytes = malloc(ar_size); + if (!ar_bytes) { + free(payload_data); + LOG_WARN("AAP2Agent: Memory allocation failed!"); + return aap2_ResponseStatus_RESPONSE_STATUS_ERROR; + } + ar_bytes[0] = 0x82; // CBOR array of length 2 CborEncoder encoder; diff --git a/components/agents/echo_agent.c b/components/agents/echo_agent.c index 24a9adb..1940679 100644 --- a/components/agents/echo_agent.c +++ b/components/agents/echo_agent.c @@ -73,6 +73,11 @@ int echo_agent_setup(struct bundle_agent_interface *const bai, sizeof(struct echo_agent_params) ); + if (!params) { + LOG_WARN("EchoAgent: Memory allocation failed!"); + return -1; + } + params->is_ipn = bai->local_admin_endpoint.scheme == EID_SCHEME_IPN; params->local_admin_endpoint = eid_dup(bai->local_admin_endpoint); params->lifetime_ms = lifetime_ms; diff --git a/components/agents/posix/application_agent.c b/components/agents/posix/application_agent.c index c8899f0..b292f75 100644 --- a/components/agents/posix/application_agent.c +++ b/components/agents/posix/application_agent.c @@ -317,6 +317,12 @@ static int16_t process_aap_message( const size_t ar_size = msg.payload_length + ar_typecode_size + 1; uint8_t *const ar_bytes = malloc(ar_size); + if (!ar_bytes) { + LOG_ERROR("AppAgent: Memory allocation failed!"); + response.type = AAP_MESSAGE_NACK; + break; + } + ar_bytes[0] = 0x82; // CBOR array of length 2 CborEncoder encoder; diff --git a/components/agents/storage/sqlite_agent.c b/components/agents/storage/sqlite_agent.c index be21825..f7d2da0 100644 --- a/components/agents/storage/sqlite_agent.c +++ b/components/agents/storage/sqlite_agent.c @@ -200,7 +200,7 @@ static enum ud3tn_result storage_operation_delete_bundles(sqlite3 *db, StorageCa static enum ud3tn_result storage_operation_push_bundles( sqlite3 *db, QueueIdentifier_t cla_queue, - StorageCall *storage_call) + const StorageCall *storage_call) { (void)db; diff --git a/components/cla/bibe_proto.c b/components/cla/bibe_proto.c index 92f26d6..40e2505 100644 --- a/components/cla/bibe_proto.c +++ b/components/cla/bibe_proto.c @@ -118,10 +118,13 @@ static void write_to_buffer( memcpy(&buffer[position], data, length); } -struct bibe_header bibe_encode_header(const char *const dest_eid, - const size_t payload_len) +int bibe_encode_header( + const char *const dest_eid, + const size_t payload_len, + struct bibe_header *hdr) { - struct bibe_header hdr; + ASSERT(hdr != NULL); + const size_t eid_len = strlen(dest_eid); // len == 8 bytes + 1 byte header const size_t CBOR_MAX_UINT_LENGTH = 9; @@ -130,6 +133,9 @@ struct bibe_header bibe_encode_header(const char *const dest_eid, uint8_t *temp_buffer = malloc(CBOR_MAX_UINT_LENGTH); CborEncoder encoder; + if (!temp_buffer) + return -1; + cbor_encoder_init(&encoder, temp_buffer, CBOR_MAX_UINT_LENGTH, 0); cbor_encode_uint(&encoder, (uint64_t)payload_len); const size_t bpdu_size = cbor_encoder_get_buffer_size( @@ -141,6 +147,11 @@ struct bibe_header bibe_encode_header(const char *const dest_eid, /* Encoding the BPDU */ uint8_t *bibe_bytes = malloc(bpdu_size); + if (!bibe_bytes) { + free(temp_buffer); + return -1; + } + bibe_bytes[0] = 0x83; // 83 (100|00011) -> Array of length 3 bibe_bytes[1] = 0x00; // 00 -> Integer 0 (transm. ID) bibe_bytes[2] = 0x00; // 00 -> Integer 0 (retr. time) @@ -163,22 +174,27 @@ struct bibe_header bibe_encode_header(const char *const dest_eid, }; // NOTE: bpdu_size is still included here - hdr.hdr_len = aap_get_serialized_size(&msg) - payload_len; - hdr.data = malloc(hdr.hdr_len); + hdr->hdr_len = aap_get_serialized_size(&msg) - payload_len; + hdr->data = malloc(hdr->hdr_len); - ASSERT(hdr.data); - ASSERT(hdr.hdr_len != 0); + if (!hdr->data) { + free(bibe_bytes); + return -1; + } - aap_serialize_into(hdr.data, &msg, false); + ASSERT(hdr->data); + ASSERT(hdr->hdr_len != 0); + + aap_serialize_into(hdr->data, &msg, false); /* Appending the BPDU to the AAP message */ write_to_buffer( - hdr.data, + hdr->data, bibe_bytes, - hdr.hdr_len - bpdu_size, + hdr->hdr_len - bpdu_size, bpdu_size ); free(bibe_bytes); - return hdr; + return 0; } diff --git a/components/cla/cla.c b/components/cla/cla.c index 9928e6f..2a4701b 100644 --- a/components/cla/cla.c +++ b/components/cla/cla.c @@ -128,6 +128,10 @@ enum ud3tn_result cla_initialize_all( return UD3TN_FAIL; char *const cla_config_str_dup = strdup(cla_config_str); + + if (!cla_config_str_dup) + return UD3TN_FAIL; + char *cur_cla_config = cla_config_str_dup; char *comma = strchr(cur_cla_config, ';'); enum ud3tn_result result = UD3TN_FAIL; @@ -242,14 +246,18 @@ enum ud3tn_result cla_link_init(struct cla_link *link, // Notify the BP task of the newly established connection... const struct bundle_agent_interface *bundle_agent_interface = config->bundle_agent_interface; + char *peer_cla_addr = cla_get_cla_addr_from_link(link); + + if (!peer_cla_addr) { + LOG_ERROR("CLA: Failed to get CLA address from link."); + goto fail_rx_task; + } bundle_processor_inform( bundle_agent_interface->bundle_signaling_queue, (struct bundle_processor_signal) { .type = BP_SIGNAL_NEW_LINK_ESTABLISHED, - .peer_cla_addr = cla_get_cla_addr_from_link( - link - ), + .peer_cla_addr = peer_cla_addr, } ); } @@ -291,13 +299,18 @@ void cla_link_cleanup(struct cla_link *link) const struct bundle_agent_interface *bundle_agent_interface = link->config->bundle_agent_interface; - bundle_processor_inform( - bundle_agent_interface->bundle_signaling_queue, - (struct bundle_processor_signal) { - .type = BP_SIGNAL_LINK_DOWN, - .peer_cla_addr = cla_get_cla_addr_from_link(link), - } - ); + char *peer_cla_addr = cla_get_cla_addr_from_link(link); + + if (peer_cla_addr) + bundle_processor_inform( + bundle_agent_interface->bundle_signaling_queue, + (struct bundle_processor_signal) { + .type = BP_SIGNAL_LINK_DOWN, + .peer_cla_addr = peer_cla_addr, + } + ); + else + LOG_ERROR("CLA: Failed to get CLA address from link."); // Clean up semaphores hal_semaphore_delete(link->rx_task_sem); @@ -358,18 +371,24 @@ char *cla_get_cla_addr_from_link(const struct cla_link *const link) const size_t result_len = cla_name_len + 1 + addr_len + 1; char *const result = malloc(result_len); - ASSERT( - snprintf(result, result_len, "%s", cla_name) == - (int64_t)cla_name_len - ); + if (!result) + return NULL; + + int written; + + written = snprintf(result, result_len, "%s", cla_name); + ASSERT(written == (int64_t)cla_name_len); + result[cla_name_len] = ':'; - if (addr) - ASSERT(snprintf( + if (addr) { + written = snprintf( result + cla_name_len + 1, result_len - 1 - cla_name_len, "%s", addr - ) == (int64_t)addr_len); + ); + ASSERT(written == (int64_t)addr_len); + } result[result_len - 1] = '\0'; return result; diff --git a/components/cla/posix/cla_bibe.c b/components/cla/posix/cla_bibe.c index 18b9c17..ce0df26 100644 --- a/components/cla/posix/cla_bibe.c +++ b/components/cla/posix/cla_bibe.c @@ -566,7 +566,11 @@ enum cla_begin_packet_result bibe_begin_packet(struct cla_link *link, struct bibe_header hdr; - hdr = bibe_encode_header(dest_eid, length); + if (bibe_encode_header(dest_eid, length, &hdr) == -1) { + LOG_ERROR("BIBE: Header encoding failed."); + link->config->vtable->cla_disconnect_handler(link); + return CLA_BEGIN_PACKET_FAIL; + } if (tcp_send_all(tcp_link->connection_socket, hdr.data, hdr.hdr_len) == -1) { diff --git a/components/cla/posix/cla_tcp_common.c b/components/cla/posix/cla_tcp_common.c index 504212d..ab98538 100644 --- a/components/cla/posix/cla_tcp_common.c +++ b/components/cla/posix/cla_tcp_common.c @@ -264,6 +264,9 @@ static void handle_established_connection( ASSERT(struct_size >= sizeof(struct cla_tcp_link)); struct cla_tcp_link *link = malloc(struct_size); + if (!link) + return; + ASSERT(!config->link); config->link = link; @@ -275,15 +278,18 @@ static void handle_established_connection( const struct bundle_agent_interface *bundle_agent_interface = config->base.base.bundle_agent_interface; - bundle_processor_inform( - bundle_agent_interface->bundle_signaling_queue, - (struct bundle_processor_signal) { - .type = BP_SIGNAL_NEW_LINK_ESTABLISHED, - .peer_cla_addr = cla_get_cla_addr_from_link( - &link->base - ), - } - ); + char *peer_cla_addr = cla_get_cla_addr_from_link(&link->base); + + if (peer_cla_addr) + bundle_processor_inform( + bundle_agent_interface->bundle_signaling_queue, + (struct bundle_processor_signal) { + .type = BP_SIGNAL_NEW_LINK_ESTABLISHED, + .peer_cla_addr = peer_cla_addr, + } + ); + else + LOG_ERROR("CLA: Failed to get CLA address from link."); cla_link_wait_cleanup(&link->base); } diff --git a/components/cla/posix/cla_tcp_util.c b/components/cla/posix/cla_tcp_util.c index ce07302..b5f18ba 100644 --- a/components/cla/posix/cla_tcp_util.c +++ b/components/cla/posix/cla_tcp_util.c @@ -70,6 +70,11 @@ char *cla_tcp_sockaddr_to_cla_addr(struct sockaddr *const sockaddr, ); char *const result = malloc(result_len); + if (!result) { + LOG_WARN("TCP: Memory allocation failed."); + return NULL; + } + snprintf( result, result_len, diff --git a/components/platform/posix/simple_queue.c b/components/platform/posix/simple_queue.c index a32bb95..ec7c31b 100644 --- a/components/platform/posix/simple_queue.c +++ b/components/platform/posix/simple_queue.c @@ -70,6 +70,12 @@ Queue_t *queueCreate(unsigned int queue_length, unsigned int item_size) // allocate enough memory to store the actual items queue->abs_start = malloc(queue->item_length * queue->item_size); + + if (!queue->abs_start) { + free(queue); + return NULL; + } + queue->abs_end = queue->abs_start + ( queue->item_length * queue->item_size); diff --git a/components/routing/compat/config_parser.c b/components/routing/compat/config_parser.c index e9ceb3b..4d264f4 100644 --- a/components/routing/compat/config_parser.c +++ b/components/routing/compat/config_parser.c @@ -37,8 +37,17 @@ static void begin_read_data_eid( struct config_parser *parser, struct endpoint_list **target) { struct endpoint_list *new_entry = malloc(sizeof(struct endpoint_list)); + + if (!new_entry) + return; + char *new_eid = malloc(DEFAULT_EID_BUFFER_SIZE * sizeof(char)); + if (!new_eid) { + free(new_entry); + return; + } + new_eid[0] = '\0'; new_entry->eid = new_eid; new_entry->next = NULL; @@ -141,6 +150,9 @@ static void begin_read_contact(struct config_parser *parser) { struct contact_list *new_entry = malloc(sizeof(struct contact_list)); + if (!new_entry) + return; + new_entry->next = NULL; new_entry->data = contact_create(parser->router_command->data); if (parser->current_contact == NULL) diff --git a/components/ud3tn/bundle.c b/components/ud3tn/bundle.c index 0251144..bc3d6be 100644 --- a/components/ud3tn/bundle.c +++ b/components/ud3tn/bundle.c @@ -301,6 +301,9 @@ struct bundle_block *bundle_block_dup(const struct bundle_block *b) while (cur_ref != NULL) { struct eid_list *new = malloc(sizeof(struct eid_list)); + if (!new) + goto err; + new->eid = eid_dup(cur_ref->eid); new->next = dup->eid_refs; dup->eid_refs = new; @@ -495,7 +498,7 @@ struct bundle_unique_identifier bundle_get_unique_identifier( }; } -void bundle_free_unique_identifier(struct bundle_unique_identifier *id) +void bundle_free_unique_identifier(const struct bundle_unique_identifier *id) { eid_free(id->source); } diff --git a/components/ud3tn/bundle_processor.c b/components/ud3tn/bundle_processor.c index a7703c8..4a42e8a 100644 --- a/components/ud3tn/bundle_processor.c +++ b/components/ud3tn/bundle_processor.c @@ -453,7 +453,7 @@ static void inform_link_status_change( cla_addr ); if (status == FIB_LINK_STATUS_DOWN) { - struct fib_link *const link = fib_lookup_cla_addr(ctx->fib, cla_addr); + const struct fib_link *const link = fib_lookup_cla_addr(ctx->fib, cla_addr); const struct fib_entry entry = { .cla_addr = cla_addr }; // After Link deletion (by request), there is no node mapping anymore, but we need diff --git a/components/ud3tn/eid.c b/components/ud3tn/eid.c index 3895194..4f8873a 100644 --- a/components/ud3tn/eid.c +++ b/components/ud3tn/eid.c @@ -125,25 +125,26 @@ char *eid_to_string2(const struct eid eid, const bool force_ipn_2_element) if (ipn_eid == NULL) return NULL; - if (localnode) - ASSERT( - snprintf( - ipn_eid, - ipn_len, - "ipn:!.%" PRIu64, - eid.ipn_eid.service_number - ) + 1 == (ssize_t)ipn_len + int written; + + if (localnode) { + written = snprintf( + ipn_eid, + ipn_len, + "ipn:!.%" PRIu64, + eid.ipn_eid.service_number ); - else - ASSERT( - snprintf( - ipn_eid, - ipn_len, - "ipn:%" PRIu64 ".%" PRIu64, - eid.ipn_eid.fully_qualified_node_number, - eid.ipn_eid.service_number - ) + 1 == (ssize_t)ipn_len + ASSERT(written + 1 == (ssize_t)ipn_len); + } else { + written = snprintf( + ipn_eid, + ipn_len, + "ipn:%" PRIu64 ".%" PRIu64, + eid.ipn_eid.fully_qualified_node_number, + eid.ipn_eid.service_number ); + ASSERT(written + 1 == (ssize_t)ipn_len); + } return ipn_eid; } else if (eid.scheme == EID_SCHEME_IPN) { // ipn 3-element format @@ -165,16 +166,15 @@ char *eid_to_string2(const struct eid eid, const bool force_ipn_2_element) if (ipn_eid == NULL) return NULL; - ASSERT( - snprintf( - ipn_eid, - ipn_len, - "ipn:%" PRIu32 ".%" PRIu32 ".%" PRIu64, - ipn_eid_allocator(eid.ipn_eid), - ipn_eid_node(eid.ipn_eid), - eid.ipn_eid.service_number - ) + 1 == (ssize_t)ipn_len + const int written = snprintf( + ipn_eid, + ipn_len, + "ipn:%" PRIu32 ".%" PRIu32 ".%" PRIu64, + ipn_eid_allocator(eid.ipn_eid), + ipn_eid_node(eid.ipn_eid), + eid.ipn_eid.service_number ); + ASSERT(written + 1 == (ssize_t)ipn_len); return ipn_eid; } else if (eid.scheme == EID_SCHEME_UNKNOWN) { return NULL; @@ -426,14 +426,13 @@ char *eid_get_agent_id_str(const struct eid eid) result = malloc(result_len + 1); // + '\0' if (!result) return NULL; - ASSERT( - snprintf( - result, - result_len + 1, - "%" PRIu64, - eid.ipn_eid.service_number - ) == (ssize_t)result_len + int written = snprintf( + result, + result_len + 1, + "%" PRIu64, + eid.ipn_eid.service_number ); + ASSERT(written == (ssize_t)result_len); return result; case EID_SCHEME_UNKNOWN: return NULL; diff --git a/include/cla/bibe_proto.h b/include/cla/bibe_proto.h index c26a673..a01a505 100644 --- a/include/cla/bibe_proto.h +++ b/include/cla/bibe_proto.h @@ -15,6 +15,6 @@ struct bibe_header { size_t bibe_parser_parse(const uint8_t *buffer, size_t length, struct bibe_protocol_data_unit *bpdu); -struct bibe_header bibe_encode_header(const char *dest_eid, size_t payload_len); +int bibe_encode_header(const char *dest_eid, size_t payload_len, struct bibe_header *hdr); #endif // CLA_BIBE_PROTO_H diff --git a/include/ud3tn/bundle.h b/include/ud3tn/bundle.h index ea2889a..aa7c1e2 100644 --- a/include/ud3tn/bundle.h +++ b/include/ud3tn/bundle.h @@ -472,7 +472,7 @@ enum ud3tn_result bundle_serialize( struct bundle_unique_identifier bundle_get_unique_identifier( const struct bundle *bundle); -void bundle_free_unique_identifier(struct bundle_unique_identifier *id); +void bundle_free_unique_identifier(const struct bundle_unique_identifier *id); bool bundle_is_equal( const struct bundle *bundle, const struct bundle_unique_identifier *id); bool bundle_is_equal_parent( diff --git a/test/unit/test_bibe_encoder.c b/test/unit/test_bibe_encoder.c index d644cf1..e9a2ab4 100644 --- a/test/unit/test_bibe_encoder.c +++ b/test/unit/test_bibe_encoder.c @@ -28,7 +28,7 @@ TEST(bibe_header_encoder, get_encoded_size) { struct bibe_header hdr; - hdr = bibe_encode_header("dtn://ud3tn.dtn", 90); + bibe_encode_header("dtn://ud3tn.dtn", 90, &hdr); TEST_ASSERT_EQUAL(EXPECTED_HEADER_LENGTH, hdr.hdr_len); @@ -39,7 +39,7 @@ TEST(bibe_header_encoder, encode_header) { struct bibe_header hdr; - hdr = bibe_encode_header("dtn://ud3tn.dtn", 90); + bibe_encode_header("dtn://ud3tn.dtn", 90, &hdr); TEST_ASSERT_EQUAL_UINT8_ARRAY( valid_header_bytes,