PengZheng commented on code in PR #459:
URL: https://github.com/apache/celix/pull/459#discussion_r1056315716
##########
bundles/deployment_admin/src/deployment_admin.c:
##########
@@ -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);
Review Comment:
missing `%s`
##########
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];
Review Comment:
Without sanity check of `maxUrlLen`, it's dangerous to use VLA. Of course,
it's an old problem.
##########
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)) {
Review Comment:
According to `man snprintf`, if an output error is encountered, a negative
value is returned.
##########
bundles/deployment_admin/src/deployment_admin.c:
##########
@@ -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);
Review Comment:
I strongly recommend using gcc attribute to perform this check
automatically, i.e. adding `__attribute__ ((format (printf, m, n)))` to all
logging related functions and adding `-Wformat` gcc option.
##########
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'
Review Comment:
`if(*netstringOut == NULL)`, it is unclear where `1024 >=
numlen+str_len+2+1`.
In `else if (*netstringMemoryOutLength < numlen + str_len + 2)`, it may
happen that `*netstringMemoryOutLength == numlen + str_len + 2`, then
`snprintf(*netstringOut, *netstringOutLength+1)` may lead to memory overwrite
by 1 byte.
##########
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);
Review Comment:
Shall we check return value of `asprintf`? According to the manual page, its
content is undefined in error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
