This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch feature/celix_err
in repository https://gitbox.apache.org/repos/asf/celix.git
The following commit(s) were added to refs/heads/feature/celix_err by this push:
new 303aca99 Refactor celix_err to use a fixed err msg buffer
303aca99 is described below
commit 303aca99528abd498eca7f61abf8870dd0471d5c
Author: Pepijn Noltes <[email protected]>
AuthorDate: Tue May 2 00:04:20 2023 +0200
Refactor celix_err to use a fixed err msg buffer
---
.../gtest/src/ArrayListErrorInjectionTestSuite.cc | 2 +
libs/utils/gtest/src/ErrTestSuite.cc | 24 +++-
libs/utils/include/celix_err.h | 11 +-
libs/utils/src/celix_err.c | 128 ++++++++++++---------
4 files changed, 105 insertions(+), 60 deletions(-)
diff --git a/libs/utils/gtest/src/ArrayListErrorInjectionTestSuite.cc
b/libs/utils/gtest/src/ArrayListErrorInjectionTestSuite.cc
index 6e17fbf8..1d3d3a7d 100644
--- a/libs/utils/gtest/src/ArrayListErrorInjectionTestSuite.cc
+++ b/libs/utils/gtest/src/ArrayListErrorInjectionTestSuite.cc
@@ -46,4 +46,6 @@ TEST_F(ArrayListErrorInjectionTestSuite, TestAddFunctions) {
//Then adding an element should fail
EXPECT_EQ(CELIX_ENOMEM, celix_arrayList_addInt(list, 10));
EXPECT_EQ(10, celix_arrayList_size(list));
+
+ celix_arrayList_destroy(list);
}
diff --git a/libs/utils/gtest/src/ErrTestSuite.cc
b/libs/utils/gtest/src/ErrTestSuite.cc
index abb15cb6..6a8c4431 100644
--- a/libs/utils/gtest/src/ErrTestSuite.cc
+++ b/libs/utils/gtest/src/ErrTestSuite.cc
@@ -35,12 +35,10 @@ TEST_F(ErrTestSuite, AddAndPopErrorTest) {
auto* m = celix_err_popLastError();
EXPECT_STREQ("error message from test", m);
- free(m);
EXPECT_EQ(1, celix_err_getErrorCount());
m = celix_err_popLastError();
EXPECT_STREQ("error message", m);
- free(m);
EXPECT_EQ(0, celix_err_getErrorCount());
}
@@ -54,3 +52,25 @@ TEST_F(ErrTestSuite, ResetErrorTest) {
EXPECT_EQ(0, celix_err_getErrorCount());
EXPECT_EQ(nullptr, celix_err_popLastError());
}
+
+TEST_F(ErrTestSuite, EdgeCaseTest) {
+ //test only "" error message, 1 char per message so max error count is
CELIX_ERR_BUFFER_SIZE
+ for (int i = 0; i < CELIX_ERR_BUFFER_SIZE + 10; ++i) {
+ celix_err_push("");
+ }
+ EXPECT_EQ(CELIX_ERR_BUFFER_SIZE, celix_err_getErrorCount());
+ for (int i = 0; i < CELIX_ERR_BUFFER_SIZE; ++i) {
+ EXPECT_STREQ("", celix_err_popLastError());
+ }
+ EXPECT_EQ(0, celix_err_getErrorCount());
+
+ //test only "[0-9]" error message, 2 char per message so max error count
is CELIX_ERR_BUFFER_SIZE/2
+ for (int i = 0; i < (CELIX_ERR_BUFFER_SIZE/2) + 10; ++i) {
+ celix_err_pushf("%i", i % 10);
+ }
+ EXPECT_EQ(CELIX_ERR_BUFFER_SIZE/2, celix_err_getErrorCount());
+ for (int i = (CELIX_ERR_BUFFER_SIZE/2) - 1; i >= 0; --i) {
+ EXPECT_STREQ(std::to_string(i % 10).c_str(), celix_err_popLastError());
+ }
+ EXPECT_EQ(0, celix_err_getErrorCount());
+}
diff --git a/libs/utils/include/celix_err.h b/libs/utils/include/celix_err.h
index 16bd2d31..7acb0f77 100644
--- a/libs/utils/include/celix_err.h
+++ b/libs/utils/include/celix_err.h
@@ -32,13 +32,20 @@ extern "C" {
* @thread_safety Thread safe.
*/
+//TODO make configurable with a cmake option
+/**
+ * @brief The error message buffer size per thread.
+ */
+#define CELIX_ERR_BUFFER_SIZE 512
+
+
/**
* @brief Returns the last error message from the current thread.
- * The error message must be freed by the caller.
+ * The error message is stored on a local thread storage and is valid until a
new error message is pushed.
* @returnval NULL if no error message is available.
*/
CELIX_UTILS_EXPORT
-char* celix_err_popLastError(); //TODO update to return const char* and remove
free
+const char* celix_err_popLastError();
/**
* @brief Push an formatted error message to the thread specific storage rcm
errors.
diff --git a/libs/utils/src/celix_err.c b/libs/utils/src/celix_err.c
index 939c7078..a6a67f11 100644
--- a/libs/utils/src/celix_err.c
+++ b/libs/utils/src/celix_err.c
@@ -23,102 +23,118 @@
#include <stdarg.h>
#include <stdio.h>
#include <pthread.h>
+#include <string.h>
-#include "celix_array_list.h"
-#include "celix_utils.h"
+pthread_key_t celix_err_tssKey; //TODO replace with celix_tss_key and
celix_tss_* functions
-pthread_key_t celix_rcm_tssErrorsKey; //TODO replace with celix_tss_key and
celix_tss_* functions
+typedef struct celix_err {
+ char buffer[CELIX_ERR_BUFFER_SIZE];
+ size_t pos;
+} celix_err_t;
-static void celix_err_destroyTssErrors(void* data) {
- //TODO replace array list with some thread specific storage char buffer
- celix_array_list_t* errors = data;
- if (errors != NULL) {
- for (int i = 0; i < celix_arrayList_size(errors); ++i) {
- char* msg = celix_arrayList_get(errors, i);
- free(msg);
+static void celix_err_destroyTssErr(void* data) {
+ celix_err_t* err = data;
+ if (err != NULL) {
+ free(err);
+ }
+}
+
+static celix_err_t* celix_err_getTssErr() {
+ celix_err_t* err = pthread_getspecific(celix_err_tssKey);
+ if (err) {
+ return err;
+ }
+
+ err = malloc(sizeof(*err));
+ if (err) {
+ err->pos = 0; //no entry
+ int rc = pthread_setspecific(celix_err_tssKey, err);
+ if (rc != 0) {
+ fprintf(stderr, "Failed to set thread specific storage for
celix_err: %s\n", strerror(rc));
+ free(err);
+ err = NULL;
}
+ } else {
+ fprintf(stderr, "Failed to allocate memory for celix_err\n");
}
- celix_arrayList_destroy(errors);
+ return err;
}
__attribute__((constructor)) void celix_err_initThreadSpecificStorageKey() {
- int rc = pthread_key_create(&celix_rcm_tssErrorsKey,
celix_err_destroyTssErrors);
+ int rc = pthread_key_create(&celix_err_tssKey, celix_err_destroyTssErr);
if (rc != 0) {
fprintf(stderr,"Failed to create thread specific storage key for
celix_err\n");
}
}
__attribute__((destructor)) void celix_err_deinitThreadSpecificStorageKey() {
- int rc = pthread_key_delete(celix_rcm_tssErrorsKey);
+ int rc = pthread_key_delete(celix_err_tssKey);
if (rc != 0) {
fprintf(stderr,"Failed to delete thread specific storage key for
celix_err\n");
}
}
-char* celix_err_popLastError() {
- char* result = NULL;
- celix_array_list_t* errors = pthread_getspecific(celix_rcm_tssErrorsKey);
- if (errors != NULL && celix_arrayList_size(errors) > 0) {
- int last = celix_arrayList_size(errors) - 1;
- result = celix_arrayList_get(errors, last);
- celix_arrayList_removeAt(errors, last);
+const char* celix_err_popLastError() {
+ const char* result = NULL;
+ celix_err_t* err = celix_err_getTssErr();
+ if (err && err->pos > 0) {
+ //move back to start last error message
+ err->pos -= 1; //move before \0 in last error message
+ while (err->pos > 0 && err->buffer[err->pos-1] != '\0') {
+ err->pos -= 1;
+ }
+ result = err->buffer + err->pos;
}
return result;
}
int celix_err_getErrorCount() {
int result = 0;
- celix_array_list_t* errors = pthread_getspecific(celix_rcm_tssErrorsKey);
- if (errors != NULL) {
- result = celix_arrayList_size(errors);
+ celix_err_t* err = celix_err_getTssErr();
+ for (int i = 0; err && i < err->pos; ++i) {
+ if (err->buffer[i] == '\0') {
+ result += 1;
+ }
}
return result;
}
void celix_err_resetErrors() {
- celix_array_list_t* errors = pthread_getspecific(celix_rcm_tssErrorsKey);
- celix_err_destroyTssErrors(errors);
- pthread_setspecific(celix_rcm_tssErrorsKey, NULL);
-}
-
-static void celix_err_pushMsg(char* msg) {
- celix_array_list_t* errors = pthread_getspecific(celix_rcm_tssErrorsKey);
- if (errors == NULL) {
- errors = celix_arrayList_create();
- if (errors == NULL) {
- fprintf(stderr, "Failed to create error list for Apache Celix rcm
lib\n");
- free(msg);
- return;
- }
- pthread_setspecific(celix_rcm_tssErrorsKey, errors);
- }
- if (errors != NULL) {
- celix_status_t rc = celix_arrayList_add(errors, msg);
- if (rc != CELIX_SUCCESS) {
- fprintf(stderr, "Failed to add error to error list for Apache
Celix rcm lib\n");
- free(msg);
- }
+ void* data = pthread_getspecific(celix_err_tssKey);
+ if (data != NULL) {
+ celix_err_t* err = data;
+ err->pos = 0; //no entry
}
}
void celix_err_push(const char* msg) {
- char* msgCpy = celix_utils_strdup(msg);
- if (msgCpy == NULL) {
- fprintf(stderr, "Failed to copy error message for Apache Celix rcm
lib\n");
- return;
+ celix_err_t* err = celix_err_getTssErr();
+ if (err) {
+ size_t len = strnlen(msg, CELIX_ERR_BUFFER_SIZE);
+ if (err->pos + len + 1 <= CELIX_ERR_BUFFER_SIZE) {
+ strcpy(err->buffer + err->pos, msg);
+ err->pos += len + 1;
+ } else {
+ fprintf(stderr, "Failed to add error message '%s' to celix_err\n",
msg);
+ }
}
- celix_err_pushMsg(msgCpy);
}
void celix_err_pushf(const char* format, ...) {
va_list args;
va_start(args, format);
- char* msg = NULL;
- int rc = vasprintf(&msg, format, args);
- if (rc >= 0) {
- celix_err_pushMsg(msg);
- } else {
- fprintf(stderr, "Failed to copy error message for Apache Celix rcm
lib\n");
+ celix_err_t* err = celix_err_getTssErr();
+ if (err) {
+ size_t len = vsnprintf(err->buffer + err->pos, CELIX_ERR_BUFFER_SIZE -
err->pos, format, args);
+ if (err->pos + len + 1 <= CELIX_ERR_BUFFER_SIZE) {
+ err->pos += len;
+ err->buffer[err->pos++] = '\0';
+ } else {
+ fprintf(stderr, "Failed to add error message '");
+ vfprintf(stderr, format, args);
+ fprintf(stderr, "' to celix_err\n");
+
+ }
}
va_end(args);
}
