This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/remove_sprintf_usage in repository https://gitbox.apache.org/repos/asf/celix.git
commit d057ce8d11a97bba8c94a758de25eff9afaaf041 Author: Pepijn Noltes <[email protected]> AuthorDate: Fri Dec 16 16:04:26 2022 +0100 Replace sprintf usage with snprintf or asprintf --- bundles/deployment_admin/src/deployment_admin.c | 8 ++++++-- .../pubsub_protocol_lib/src/pubsub_wire_protocol_common.c | 5 +++-- .../pubsub_protocol_wire_v1/gtest/src/PS_WP_tests.cc | 3 ++- .../discovery_common/include/endpoint_discovery_server.h | 2 +- .../remote_services/discovery_common/src/discovery_activator.c | 10 ++-------- .../discovery_common/src/endpoint_discovery_server.c | 8 ++++---- bundles/remote_services/discovery_etcd/src/etcd_watcher.c | 2 +- .../topology_manager/tms_tst/disc_mock/disc_mock_activator.c | 10 ++-------- libs/dfi/src/avrobin_serializer.c | 4 ++-- libs/utils/private/test/array_list_test.cpp | 4 ++-- libs/utils/private/test/hash_map_test.cpp | 5 ++--- 11 files changed, 27 insertions(+), 34 deletions(-) diff --git a/bundles/deployment_admin/src/deployment_admin.c b/bundles/deployment_admin/src/deployment_admin.c index 45eca1ab..6284e764 100644 --- a/bundles/deployment_admin/src/deployment_admin.c +++ b/bundles/deployment_admin/src/deployment_admin.c @@ -199,8 +199,10 @@ static celix_status_t deploymentAdmin_performRequest(deployment_admin_pt admin, fw_log(celix_frameworkLogger_globalLogger(), CELIX_LOG_LEVEL_ERROR, "Error initializing curl."); } - char url[strlen(admin->auditlogUrl)+6]; - sprintf(url, "%s/send", admin->auditlogUrl); + size_t maxUrlLen = strlen(admin->auditlogUrl)+6; + char url[maxUrlLen]; + int written = snprintf(url, sizeof(url), "%s/send", admin->auditlogUrl); + status = written < sizeof(url) ? CELIX_SUCCESS : CELIX_ILLEGAL_ARGUMENT; if (status == CELIX_SUCCESS) { curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1); @@ -212,6 +214,8 @@ static celix_status_t deploymentAdmin_performRequest(deployment_admin_pt admin, status = CELIX_BUNDLE_EXCEPTION; fw_log(celix_frameworkLogger_globalLogger(), CELIX_LOG_LEVEL_ERROR, "Error sending auditlog, got curl error code %d", res); } + } else { + fw_log(celix_frameworkLogger_globalLogger(), CELIX_LOG_LEVEL_ERROR, "Error creating send url for audit log url", admin->auditlogUrl); } return status; diff --git a/bundles/pubsub/pubsub_protocol/pubsub_protocol_lib/src/pubsub_wire_protocol_common.c b/bundles/pubsub/pubsub_protocol/pubsub_protocol_lib/src/pubsub_wire_protocol_common.c index 1d123d7b..cb4139d4 100644 --- a/bundles/pubsub/pubsub_protocol/pubsub_protocol_lib/src/pubsub_wire_protocol_common.c +++ b/bundles/pubsub/pubsub_protocol/pubsub_protocol_lib/src/pubsub_wire_protocol_common.c @@ -62,8 +62,9 @@ static celix_status_t pubsubProtocol_createNetstring(const char* string, char** return CELIX_ENOMEM; } } - *netstringOutLength = numlen + str_len + 2; - sprintf(*netstringOut, "%zu:%s,", str_len, string); + *netstringOutLength = numlen + str_len + 2; //Note +2 for ':', ',' + int written = snprintf(*netstringOut, *netstringOutLength+1, "%zu:%s,", str_len, string); //adding +1 for '\0' + status = written < *netstringMemoryOutLength+1 ? CELIX_SUCCESS : CELIX_ILLEGAL_ARGUMENT; } return status; diff --git a/bundles/pubsub/pubsub_protocol/pubsub_protocol_wire_v1/gtest/src/PS_WP_tests.cc b/bundles/pubsub/pubsub_protocol/pubsub_protocol_wire_v1/gtest/src/PS_WP_tests.cc index c39604a2..0deae811 100644 --- a/bundles/pubsub/pubsub_protocol/pubsub_protocol_wire_v1/gtest/src/PS_WP_tests.cc +++ b/bundles/pubsub/pubsub_protocol/pubsub_protocol_wire_v1/gtest/src/PS_WP_tests.cc @@ -172,6 +172,7 @@ TEST_F(WireProtocolV1Test, WireProtocolV1Test_EncodeMetadata_Test) { // NOLINT(c void *data = nullptr; size_t length = 0; celix_status_t status = pubsubProtocol_v1_encodeMetadata(nullptr, &message, &data, &length); + ASSERT_EQ(status, CELIX_SUCCESS); unsigned char exp[12]; uint32_t s = htonl(1); @@ -181,7 +182,7 @@ TEST_F(WireProtocolV1Test, WireProtocolV1Test_EncodeMetadata_Test) { // NOLINT(c ASSERT_EQ(status, CELIX_SUCCESS); ASSERT_EQ(12, length); for (int i = 0; i < 12; i++) { - ASSERT_EQ(((unsigned char*) data)[i], exp[i]); + ASSERT_EQ(((unsigned char*) data)[i], exp[i]) << "Error at index " << i; } celix_properties_destroy(message.metadata.metadata); diff --git a/bundles/remote_services/discovery_common/include/endpoint_discovery_server.h b/bundles/remote_services/discovery_common/include/endpoint_discovery_server.h index cf72eabf..7bf05839 100644 --- a/bundles/remote_services/discovery_common/include/endpoint_discovery_server.h +++ b/bundles/remote_services/discovery_common/include/endpoint_discovery_server.h @@ -81,7 +81,7 @@ celix_status_t endpointDiscoveryServer_removeEndpoint(endpoint_discovery_server_ * @param url [out] url which is used to announce the endpoints. * @return CELIX_SUCCESS when successful. */ -celix_status_t endpointDiscoveryServer_getUrl(endpoint_discovery_server_t *server, char* url); +celix_status_t endpointDiscoveryServer_getUrl(endpoint_discovery_server_t *server, char* url, size_t maxLenUrl); #endif /* ENDPOINT_DISCOVERY_SERVER_H_ */ diff --git a/bundles/remote_services/discovery_common/src/discovery_activator.c b/bundles/remote_services/discovery_common/src/discovery_activator.c index 2c54d835..c7f7243f 100644 --- a/bundles/remote_services/discovery_common/src/discovery_activator.c +++ b/bundles/remote_services/discovery_common/src/discovery_activator.c @@ -102,14 +102,8 @@ celix_status_t bundleActivator_start(void * userData, celix_bundle_context_t *co return CELIX_ILLEGAL_STATE; } - size_t len = 11 + strlen(OSGI_FRAMEWORK_OBJECTCLASS) + strlen(OSGI_RSA_ENDPOINT_FRAMEWORK_UUID) + strlen(uuid); - char *scope = malloc(len + 1); - if (!scope) { - return CELIX_ENOMEM; - } - - sprintf(scope, "(&(%s=*)(%s=%s))", OSGI_FRAMEWORK_OBJECTCLASS, OSGI_RSA_ENDPOINT_FRAMEWORK_UUID, uuid); - scope[len] = 0; + char* scope = NULL; + asprintf(&scope, "(&(%s=*)(%s=%s))", OSGI_FRAMEWORK_OBJECTCLASS, OSGI_RSA_ENDPOINT_FRAMEWORK_UUID, uuid); celix_logHelper_debug(activator->loghelper, "using scope %s.", scope); diff --git a/bundles/remote_services/discovery_common/src/endpoint_discovery_server.c b/bundles/remote_services/discovery_common/src/endpoint_discovery_server.c index d09ca2af..41725d03 100644 --- a/bundles/remote_services/discovery_common/src/endpoint_discovery_server.c +++ b/bundles/remote_services/discovery_common/src/endpoint_discovery_server.c @@ -209,13 +209,13 @@ celix_status_t endpointDiscoveryServer_create(discovery_t *discovery, return status; } -celix_status_t endpointDiscoveryServer_getUrl(endpoint_discovery_server_t *server, char* url) -{ +celix_status_t endpointDiscoveryServer_getUrl(endpoint_discovery_server_t *server, char* url, size_t maxLenUrl) { + celix_status_t status = CELIX_BUNDLE_EXCEPTION; if (server->ip && server->port && server->path) { - sprintf(url, "http://%s:%s/%s";, server->ip, server->port, server->path); - status = CELIX_SUCCESS; + int written = snprintf(url, maxLenUrl, "http://%s:%s/%s";, server->ip, server->port, server->path); + status = written < maxLenUrl ? CELIX_SUCCESS : CELIX_ILLEGAL_ARGUMENT; } return status; diff --git a/bundles/remote_services/discovery_etcd/src/etcd_watcher.c b/bundles/remote_services/discovery_etcd/src/etcd_watcher.c index c7ada82a..b3d6c70c 100644 --- a/bundles/remote_services/discovery_etcd/src/etcd_watcher.c +++ b/bundles/remote_services/discovery_etcd/src/etcd_watcher.c @@ -148,7 +148,7 @@ static celix_status_t etcdWatcher_addOwnFramework(etcd_watcher_t *watcher) return status; } - if (endpointDiscoveryServer_getUrl(server, url) != CELIX_SUCCESS) { + if (endpointDiscoveryServer_getUrl(server, url, MAX_VALUE_LENGTH) != CELIX_SUCCESS) { snprintf(url, MAX_VALUE_LENGTH, "http://%s:%s/%s";, DEFAULT_SERVER_IP, DEFAULT_SERVER_PORT, DEFAULT_SERVER_PATH); } diff --git a/bundles/remote_services/topology_manager/tms_tst/disc_mock/disc_mock_activator.c b/bundles/remote_services/topology_manager/tms_tst/disc_mock/disc_mock_activator.c index 40e26f33..7b1da874 100644 --- a/bundles/remote_services/topology_manager/tms_tst/disc_mock/disc_mock_activator.c +++ b/bundles/remote_services/topology_manager/tms_tst/disc_mock/disc_mock_activator.c @@ -72,14 +72,8 @@ celix_status_t bundleActivator_start(void * userData, celix_bundle_context_t *co return CELIX_ILLEGAL_STATE; } - size_t len = 11 + strlen(OSGI_FRAMEWORK_OBJECTCLASS) + strlen(OSGI_RSA_ENDPOINT_FRAMEWORK_UUID) + strlen(uuid); - char *scope = malloc(len + 1); - if (!scope) { - return CELIX_ENOMEM; - } - - sprintf(scope, "(&(%s=*)(%s=%s))", OSGI_FRAMEWORK_OBJECTCLASS, OSGI_RSA_ENDPOINT_FRAMEWORK_UUID, uuid); - scope[len] = 0; + char* scope = NULL; + asprintf(&scope, "(&(%s=*)(%s=%s))", OSGI_FRAMEWORK_OBJECTCLASS, OSGI_RSA_ENDPOINT_FRAMEWORK_UUID, uuid); celix_properties_t *props = celix_properties_create(); celix_properties_set(props, "DISCOVERY", "true"); diff --git a/libs/dfi/src/avrobin_serializer.c b/libs/dfi/src/avrobin_serializer.c index ef6f6f6f..328a343b 100644 --- a/libs/dfi/src/avrobin_serializer.c +++ b/libs/dfi/src/avrobin_serializer.c @@ -692,7 +692,7 @@ static int avrobinSerializer_writeSequence(dyn_type *type, void *loc, FILE *stre static int avrobinSerializer_writeEnum(dyn_type *type, void *loc, FILE *stream) { char enum_value_str[16]; - if (sprintf(enum_value_str, "%d", *(int32_t*)loc) < 0) { + if (snprintf(enum_value_str, sizeof(enum_value_str), "%d", *(int32_t*)loc) >= sizeof(enum_value_str)) { return ERROR; } @@ -1101,7 +1101,7 @@ static int generate_sync(uint8_t **result) { static int generate_record_name(char **result) { char num_str[16]; - if (sprintf(num_str, "R%u", record_name_counter) < 0) { + if (snprintf(num_str, sizeof(num_str), "R%u", record_name_counter) >= sizeof(num_str)) { return ERROR; } size_t len = strlen(num_str); diff --git a/libs/utils/private/test/array_list_test.cpp b/libs/utils/private/test/array_list_test.cpp index cb5f652c..da90de5a 100644 --- a/libs/utils/private/test/array_list_test.cpp +++ b/libs/utils/private/test/array_list_test.cpp @@ -127,7 +127,7 @@ TEST(array_list, clone) { for (i = 0; i < 12; i++) { bool added; char entry[11]; - sprintf(entry, "|%s|%d|", "entry", i); + snprintf(entry, sizeof(entry), "|%s|%d|", "entry", i); added = arrayList_add(list, my_strdup(entry)); CHECK(added); } @@ -144,7 +144,7 @@ TEST(array_list, clone) { for (j = 0; j < 12; j++) { void *entry = NULL; char entrys[11]; - sprintf(entrys, "|%s|%d|", "entry", j); + snprintf(entrys, sizeof(entrys), "|%s|%d|", "entry", j); entry = arrayList_get(clone, j); STRCMP_EQUAL((char *) entry, entrys); diff --git a/libs/utils/private/test/hash_map_test.cpp b/libs/utils/private/test/hash_map_test.cpp index 428777fd..6f657eb4 100644 --- a/libs/utils/private/test/hash_map_test.cpp +++ b/libs/utils/private/test/hash_map_test.cpp @@ -390,15 +390,14 @@ TEST(hash_map, resize){ LONGS_EQUAL(16, map->tablelength); LONGS_EQUAL(12, map->treshold); for (i = 0; i < 12; i++) { - char key[6]; - sprintf(key, "key%d", i); + snprintf(key, sizeof(key), "key%d", i); hashMap_put(map, my_strdup(key), my_strdup(key)); } LONGS_EQUAL(12, map->size); LONGS_EQUAL(16, map->tablelength); LONGS_EQUAL(12, map->treshold); - sprintf(key, "key%d", i); + snprintf(key, sizeof(key), "key%d", i); hashMap_put(map, my_strdup(key), my_strdup(key)); LONGS_EQUAL(13, map->size); LONGS_EQUAL(32, map->tablelength);
