apacheGH-491: Check return value of vasprintf and asprintf #781
Conversation
|
@PengZheng |
bundles/event_admin/remote_provider/remote_provider_mqtt/src/celix_earpm_impl.c
Outdated
Show resolved
Hide resolved
|
@xuzhenbao @PengZheng |
As mentioned before, the framework part is undergoing active refactor, see #779 for a recent example.
Please note that conflict should be (and can only be) resolved in your own branch. |
| rc= asprintf(&port_str, "%li", port); | ||
| if(rc < 0 || port_str == NULL) { | ||
| celix_bundleContext_log(ctx, CELIX_LOG_LEVEL_ERROR, "Cannot allocate memory for port string."); | ||
| free(httpRoot); |
| @@ -147,10 +147,12 @@ celix_event_admin_remote_provider_mqtt_t* celix_earpm_create(celix_bundle_contex | |||
| return NULL; | |||
| } | |||
|
|
|||
| if (asprintf(&earpm->syncEventAckTopic, CELIX_EARPM_SYNC_EVENT_ACK_TOPIC_PREFIX"%s", earpm->fwUUID) < 0) { | |||
| earpm->syncEventAckTopic = NULL; | |||
There was a problem hiding this comment.
Given that earpm is calloced, it is unnecessary to set it to NULL explicitly.
There was a problem hiding this comment.
It seems that you need to familiarize yourself with the code base before making changes, which already introduced memory safety bugs.
It may help to learn how to add tests for your changes, especially the error injector tech. The double-free issue could be easily caught by such tests with the help of AddressSanitizer.
Rationale for this change
The change addresses potential memory management issues in the discovery server, ensuring proper resource allocation, error handling, and cleanup to enhance stability and prevent crashes or memory leaks.
What changes are included in this PR?
malloc,strdup,asprintf) with appropriate error handling.Are these changes tested?
These changes are tested with existing functionality.
Are there any user-facing changes?
No user-facing changes. These improvements focus on internal stability and reliability.