This is an automated email from the ASF dual-hosted git repository.
pengzheng pushed a commit to branch feature/refactor_bundle_cache
in repository https://gitbox.apache.org/repos/asf/celix.git
The following commit(s) were added to refs/heads/feature/refactor_bundle_cache
by this push:
new b98fc0d4 Avoid memory allocation in `celix_utils_extractZipInternal`
and improve `celix_framework_logCode`.
b98fc0d4 is described below
commit b98fc0d4717ad7d453d25013b407b25558590582
Author: PengZheng <[email protected]>
AuthorDate: Mon Mar 27 21:55:20 2023 +0800
Avoid memory allocation in `celix_utils_extractZipInternal` and improve
`celix_framework_logCode`.
---
libs/framework/gtest/src/CelixFrameworkUtilsTestSuite.cc | 5 +++++
libs/framework/src/celix_framework_utils.c | 7 ++-----
libs/framework/src/celix_log.c | 12 ++++++------
libs/utils/include/celix_utils.h | 13 +++++++++++++
libs/utils/src/celix_file_utils.c | 7 +++----
libs/utils/src/utils.c | 16 ++++++++++++----
6 files changed, 41 insertions(+), 19 deletions(-)
diff --git a/libs/framework/gtest/src/CelixFrameworkUtilsTestSuite.cc
b/libs/framework/gtest/src/CelixFrameworkUtilsTestSuite.cc
index 94d7cea7..9d5b2cea 100644
--- a/libs/framework/gtest/src/CelixFrameworkUtilsTestSuite.cc
+++ b/libs/framework/gtest/src/CelixFrameworkUtilsTestSuite.cc
@@ -84,6 +84,11 @@ TEST_F(CelixFrameworkUtilsTestSuite, testExtractBundlePath) {
status = celix_framework_utils_extractBundle(framework->getCFramework(),
"non-existing.zip", testExtractDir); //note nullptr framwork is allowed,
fallback to global logger.
EXPECT_NE(status, CELIX_SUCCESS);
+
+ //invalid bundle path -> no extraction
+ status = celix_framework_utils_extractBundle(framework->getCFramework(),
"./", testExtractDir); //note nullptr framwork is allowed, fallback to global
logger.
+ EXPECT_NE(status, CELIX_SUCCESS);
+
//invalid url prefix -> no extraction
std::string path = std::string{"bla://"} + SIMPLE_TEST_BUNDLE1_LOCATION;
status = celix_framework_utils_extractBundle(framework->getCFramework(),
path.c_str(), testExtractDir);
diff --git a/libs/framework/src/celix_framework_utils.c
b/libs/framework/src/celix_framework_utils.c
index 13fe73d8..76ec207b 100644
--- a/libs/framework/src/celix_framework_utils.c
+++ b/libs/framework/src/celix_framework_utils.c
@@ -169,9 +169,7 @@ static bool
celix_framework_utils_extractBundlePath(celix_framework_t *fw, const
char* resolvedPath = celix_framework_utils_resolveFileBundleUrl(buffer,
sizeof(buffer), fw, bundlePath, false);
assert(resolvedPath != NULL); //should be caught by
celix_framework_utils_isBundleUrlValid
celix_status_t status = celix_utils_extractZipFile(resolvedPath,
extractPath, &err);
- if (status != CELIX_SUCCESS) {
- FW_LOG(CELIX_LOG_LEVEL_ERROR, "Could not extract bundle zip file `%s`
to `%s`: %s", resolvedPath, extractPath, err);
- }
+ framework_logIfError(fw->logger, status, err, "Could not extract bundle
zip file `%s` to `%s`", resolvedPath, extractPath);
celix_utils_freeStringIfNotEqual(buffer, resolvedPath);
return status == CELIX_SUCCESS;
}
@@ -201,9 +199,8 @@ static bool
celix_framework_utils_extractBundleEmbedded(celix_framework_t *fw, c
celix_status_t status = celix_utils_extractZipData(start, end-start,
extractPath, &err);
if (status == CELIX_SUCCESS) {
FW_LOG(CELIX_LOG_LEVEL_TRACE, "Embedded bundle zip `%s` extracted to
`%s`", embeddedBundle, extractPath);
- } else {
- FW_LOG(CELIX_LOG_LEVEL_ERROR, "Could not extract embedded bundle zip
`%s` to `%s`: %s", embeddedBundle, extractPath, err);
}
+ framework_logIfError(fw->logger, status, err, "Could not extract embedded
bundle zip `%s` to `%s`", embeddedBundle, extractPath);
return status == CELIX_SUCCESS;
}
diff --git a/libs/framework/src/celix_log.c b/libs/framework/src/celix_log.c
index 429d06e5..4c3bc530 100644
--- a/libs/framework/src/celix_log.c
+++ b/libs/framework/src/celix_log.c
@@ -103,7 +103,7 @@ void
celix_frameworkLogger_setLogCallback(celix_framework_logger_t* logger, void
}
-static void celix_framework_vlogInternal(celix_framework_logger_t* logger,
celix_log_level_e level, celix_status_t *optionalStatus, const char* file,
const char* function, int line, const char* format, va_list args) {
+static void celix_framework_vlogInternal(celix_framework_logger_t* logger,
celix_log_level_e level, celix_status_t optionalStatus, const char* file, const
char* function, int line, const char* format, va_list args) {
if (level == CELIX_LOG_LEVEL_DISABLED) {
return;
}
@@ -112,9 +112,9 @@ static void
celix_framework_vlogInternal(celix_framework_logger_t* logger, celix
//note let log function handle active log levels
logger->logFunction(logger->logHandle, level, file, function, line,
format, args);
} else if (level >= logger->activeLogLevel) {
- if (optionalStatus != NULL) {
+ if (optionalStatus != CELIX_SUCCESS) {
fseek(logger->stream, 0L, SEEK_SET);
- fprintf(logger->stream, "%s: ", celix_strerror(*optionalStatus));
+ fprintf(logger->stream, "%s(0x%x): ",
celix_strerror(optionalStatus), optionalStatus);
vfprintf(logger->stream, format, args);
fputc('\0', logger->stream); //note not sure if this is needed
fflush(logger->stream);
@@ -128,20 +128,20 @@ static void
celix_framework_vlogInternal(celix_framework_logger_t* logger, celix
void celix_framework_vlog(celix_framework_logger_t* logger, celix_log_level_e
level, const char *func, const char *file, int line,
const char *format, va_list formatArgs) {
- celix_framework_vlogInternal(logger, level, NULL, func, file, line,
format, formatArgs);
+ celix_framework_vlogInternal(logger, level, CELIX_SUCCESS, func, file,
line, format, formatArgs);
}
void celix_framework_log(celix_framework_logger_t* logger, celix_log_level_e
level, const char *func, const char *file, int line, const char *format, ...) {
va_list args;
va_start(args, format);
- celix_framework_vlogInternal(logger, level, NULL, file, func, line,
format, args);
+ celix_framework_vlogInternal(logger, level, CELIX_SUCCESS, file, func,
line, format, args);
va_end(args);
}
void celix_framework_logCode(celix_framework_logger_t* logger,
celix_log_level_e level, const char *func, const char *file, int line,
celix_status_t code, const char *format, ...) {
va_list args;
va_start(args, format);
- celix_framework_vlogInternal(logger, level, &code, file, func, line,
format, args);
+ celix_framework_vlogInternal(logger, level, code, file, func, line,
format, args);
va_end(args);
}
\ No newline at end of file
diff --git a/libs/utils/include/celix_utils.h b/libs/utils/include/celix_utils.h
index 84ed57e8..0b9b17d0 100644
--- a/libs/utils/include/celix_utils.h
+++ b/libs/utils/include/celix_utils.h
@@ -25,6 +25,7 @@ extern "C" {
#endif
#include <time.h>
+#include <stdarg.h>
#include <stdbool.h>
#define CELIX_UTILS_MAX_STRLEN 1024*1024*1024
@@ -63,6 +64,18 @@ unsigned int celix_utils_stringHash(const char* string);
char* celix_utils_writeOrCreateString(char* buffer, size_t bufferSize, const
char* format, ...)
__attribute__((format(printf, 3, 4)));
+/**
+ * @brief Format a string to the provided buffer or a newly allocated buffer
if the provided buffer is to small.
+ * @param[in,out] buffer The buffer to write the formatted string to.
+ * @param[in] bufferSize The size of the buffer.
+ * @param[in] format The format string.
+ * @param[in] formatArgs The arguments for the format string.
+ * @return The formatted string in the provided buffer or a newly allocated
buffer if the provided buffer is to small.
+ * @retval NULL if a allocation was needed, but failed.
+ */
+char* celix_utils_writeOrCreateVString(char* buffer, size_t bufferSize, const
char* format, va_list formatArgs)
+__attribute__((format(printf, 3, 0)));
+
/**
* @brief Free the provided str if the str is not equal to the provided buffer.
* @note This function is useful combined with celix_utils_writeOrCreateString.
diff --git a/libs/utils/src/celix_file_utils.c
b/libs/utils/src/celix_file_utils.c
index 7bb324a2..c930618c 100644
--- a/libs/utils/src/celix_file_utils.c
+++ b/libs/utils/src/celix_file_utils.c
@@ -190,8 +190,7 @@ static celix_status_t celix_utils_extractZipInternal(zip_t
*zip, const char* ext
zip_stat_t st;
zip_stat_index(zip, i, 0, &st);
- char* path = NULL;
- asprintf(&path, "%s/%s", extractToDir, st.name);
+ char* path = celix_utils_writeOrCreateString(buf, bufSize, "%s/%s",
extractToDir, st.name);
if (st.name[strlen(st.name) - 1] == '/') {
status = celix_utils_createDirectory(path, false, errorOut);
} else {
@@ -220,7 +219,7 @@ static celix_status_t celix_utils_extractZipInternal(zip_t
*zip, const char* ext
zip_fclose(zf);
}
}
- free(path);
+ celix_utils_freeStringIfNotEqual(buf, path);
}
return status;
}
@@ -272,7 +271,7 @@ celix_status_t celix_utils_extractZipData(const void
*zipData, size_t zipDataSiz
if (source == NULL || zip == NULL) {
status = CELIX_FILE_IO_EXCEPTION;
- *errorOut = zip_error_strerror(&zipError);
+ *errorOut = ERROR_OPENING_ZIP;
}
if (zip != NULL) {
zip_close(zip);
diff --git a/libs/utils/src/utils.c b/libs/utils/src/utils.c
index 0e2dea71..da17f1a4 100644
--- a/libs/utils/src/utils.c
+++ b/libs/utils/src/utils.c
@@ -173,19 +173,27 @@ bool utils_isStringEmptyOrNull(const char * const str) {
}
char* celix_utils_writeOrCreateString(char* buffer, size_t bufferSize, const
char* format, ...) {
+ char *ret = NULL;
va_list args;
va_start(args, format);
- int written = vsnprintf(buffer, bufferSize, format, args);
+ ret = celix_utils_writeOrCreateVString(buffer, bufferSize, format, args);
va_end(args);
+ return ret;
+}
+
+char* celix_utils_writeOrCreateVString(char* buffer, size_t bufferSize, const
char* format, va_list formatArgs) {
+ va_list argCopy;
+ va_copy(argCopy, formatArgs);
+ int written = vsnprintf(buffer, bufferSize, format, argCopy);
+ va_end(argCopy);
if (written < 0 || written >= bufferSize) {
//buffer to small, create new string
char* newStr = NULL;
- va_start(args, format);
- int rc = vasprintf(&newStr, format, args);
- va_end(args);
+ int rc = vasprintf(&newStr, format, formatArgs);
return rc == -1 ? NULL : newStr;
}
return buffer;
+
}
void celix_utils_freeStringIfNotEqual(const char* buffer, char* str) {
