PengZheng commented on code in PR #796:
URL: https://github.com/apache/celix/pull/796#discussion_r2250234036
##########
documents/development/README.md:
##########
@@ -381,37 +380,50 @@ For log levels use the following guidelines:
- fatal: Use this level to report severe errors that prevent the program from
continuing to run.
After logging a fatal error, the program will typically terminate.
-Example of error handling and logging:
+Example of error handling and logging using auto pointers:
```c
+typedef struct celix_foo {
+ celix_thread_mutex_t mutex;
+ bool mutexInitialized;
+ celix_array_list_t* list;
+ celix_long_hash_map_t* map;
+} celix_foo_t;
+
+CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_foo_t, celix_foo_destroy)
+
celix_foo_t* celix_foo_create(celix_log_helper_t* logHelper) {
- celix_foo_t* foo = calloc(1, sizeof(*foo));
+ celix_autoptr(celix_foo_t) foo = calloc(1, sizeof(*foo));
if (!foo) {
- goto create_enomem_err;
+ celix_logHelper_log(logHelper, CELIX_LOG_LEVEL_ERROR,
+ "Error creating foo, out of memory");
+ return NULL;
}
-
- CELIX_GOTO_IF_ERR(create_mutex_err, celixThreadMutex_create(&foo->mutex,
NULL));
-
+
+ if (celixThreadMutex_create(&foo->mutex, NULL) == CELIX_SUCCESS) {
+ foo->mutexInitialized = true;
+ } else {
+ celix_logHelper_log(logHelper, CELIX_LOG_LEVEL_ERROR,
+ "Error creating mutex");
+ return NULL; //foo cleaned up automatically
+ }
+
foo->list = celix_arrayList_create();
foo->map = celix_longHashMap_create();
- if (!foo->list || !foo->map) {
- goto create_enomem_err;
+ if (!foo->list || !foo->map) {
+ celix_logHelper_log(logHelper, CELIX_LOG_LEVEL_ERROR,
+ "Error creating foo, out of memory");
+ return NULL; //foo cleaned up automatically
}
-
- return foo;
-create_mutex_err:
- celix_logHelper_log(logHelper, CELIX_LOG_LEVEL_ERROR, "Error creating
mutex");
- free(foo); //mutex not created, do not use celix_foo_destroy to prevent
mutex destroy
- return NULL;
-create_enomem_err:
- celix_logHelper_log(logHelper, CELIX_LOG_LEVEL_ERROR, "Error creating foo,
out of memory");
- celix_foo_destroy(foo); //note celix_foo_destroy can handle NULL
- return NULL;
+
+ return celix_steal_ptr(foo);
}
void celix_foo_destroy(celix_foo_t* foo) {
if (foo != NULL) {
//note reverse order of creation
- celixThreadMutex_destroy(&foo->mutex);
+ if (foo->mutexInitialized) {
Review Comment:
Half-initialized objects would better be avoided.
If `celix_foo_create` failed, the caller will get nullptr.
##########
documents/development/README.md:
##########
@@ -381,37 +380,50 @@ For log levels use the following guidelines:
- fatal: Use this level to report severe errors that prevent the program from
continuing to run.
After logging a fatal error, the program will typically terminate.
-Example of error handling and logging:
+Example of error handling and logging using auto pointers:
```c
+typedef struct celix_foo {
+ celix_thread_mutex_t mutex;
+ bool mutexInitialized;
+ celix_array_list_t* list;
+ celix_long_hash_map_t* map;
+} celix_foo_t;
+
+CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_foo_t, celix_foo_destroy)
+
celix_foo_t* celix_foo_create(celix_log_helper_t* logHelper) {
- celix_foo_t* foo = calloc(1, sizeof(*foo));
+ celix_autoptr(celix_foo_t) foo = calloc(1, sizeof(*foo));
if (!foo) {
- goto create_enomem_err;
+ celix_logHelper_log(logHelper, CELIX_LOG_LEVEL_ERROR,
+ "Error creating foo, out of memory");
+ return NULL;
}
-
- CELIX_GOTO_IF_ERR(create_mutex_err, celixThreadMutex_create(&foo->mutex,
NULL));
-
+
+ if (celixThreadMutex_create(&foo->mutex, NULL) == CELIX_SUCCESS) {
+ foo->mutexInitialized = true;
+ } else {
+ celix_logHelper_log(logHelper, CELIX_LOG_LEVEL_ERROR,
+ "Error creating mutex");
+ return NULL; //foo cleaned up automatically
+ }
+
foo->list = celix_arrayList_create();
foo->map = celix_longHashMap_create();
- if (!foo->list || !foo->map) {
- goto create_enomem_err;
+ if (!foo->list || !foo->map) {
+ celix_logHelper_log(logHelper, CELIX_LOG_LEVEL_ERROR,
Review Comment:
But `list` and `mutex` are leaked.
We'd better manage them via SBRM, steal and store them in `foo` in the very
end.
--
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
