Eliminate GOTOs in discovery_zeroconf.

bundles/remote_services/discovery_zeroconf/gtest/src/DiscoveryZeroconfWatcherTestSuite.cc

@@ -122,6 +122,19 @@ TEST_F(DiscoveryZeroconfWatcherTestSuite, CreateAndDestroyWatcher) {
     discoveryZeroconfWatcher_destroy(watcher);
 }
 
+TEST_F(DiscoveryZeroconfWatcherTestSuite, Cleanup) {
+    celix_autoptr(discovery_zeroconf_watcher_t) watcher = nullptr;
+    celix_status_t status = discoveryZeroconfWatcher_create(ctx.get(), logHelper.get(), &watcher);
+    EXPECT_EQ(CELIX_SUCCESS, status);
+}
+
+TEST_F(DiscoveryZeroconfWatcherTestSuite, StealPointer) {
+    celix_autoptr(discovery_zeroconf_watcher_t) watcher = nullptr;
+    celix_status_t status = discoveryZeroconfWatcher_create(ctx.get(), logHelper.get(), &watcher);
+    EXPECT_EQ(CELIX_SUCCESS, status);
+    discoveryZeroconfWatcher_destroy(celix_steal_ptr(watcher));
+}
+
 TEST_F(DiscoveryZeroconfWatcherTestSuite, CreateWatcherFailed1) {
     discovery_zeroconf_watcher_t *watcher;
     celix_ei_expect_celix_bundleContext_getProperty((void*)&discoveryZeroconfWatcher_create, 0, nullptr);

bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_activator.c

@@ -31,26 +31,24 @@ typedef struct discovery_zeroconf_activator {
 
 celix_status_t discoveryZeroconfActivator_start(discovery_zeroconf_activator_t *act, celix_bundle_context_t *ctx) {
     celix_status_t status = CELIX_SUCCESS;
-    act->logHelper = celix_logHelper_create(ctx,"celix_rsa_zeroconf_discovery");
-    if (act->logHelper == NULL) {
-        status = CELIX_ENOMEM;
-        goto log_helper_err;
+    celix_autoptr(celix_log_helper_t) logger = celix_logHelper_create(ctx,"celix_rsa_zeroconf_discovery");
+    if (logger == NULL) {
+        return CELIX_ENOMEM;
     }
-    status = discoveryZeroconfAnnouncer_create(ctx, act->logHelper, &act->announcer);
+    celix_autoptr(discovery_zeroconf_announcer_t) announcer = NULL;
+    status = discoveryZeroconfAnnouncer_create(ctx, logger, &announcer);
     if (status != CELIX_SUCCESS) {
-        goto announcer_err;
+        return status;
     }
-    status = discoveryZeroconfWatcher_create(ctx, act->logHelper, &act->watcher);
+    celix_autoptr(discovery_zeroconf_watcher_t) watcher = NULL;
+    status = discoveryZeroconfWatcher_create(ctx, logger, &watcher);
     if (status != CELIX_SUCCESS) {
-        goto watcher_err;
+        return status;
     }
+    act->watcher = celix_steal_ptr(watcher);
+    act->announcer = celix_steal_ptr(announcer);
+    act->logHelper = celix_steal_ptr(logger);
     return CELIX_SUCCESS;
-watcher_err:
-    discoveryZeroconfAnnouncer_destroy(act->announcer);
-announcer_err:
-    celix_logHelper_destroy(act->logHelper);
-log_helper_err:
-    return status;
 }
 
 celix_status_t discoveryZeroconfActivator_stop(discovery_zeroconf_activator_t *act, celix_bundle_context_t *ctx) {

bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_announcer.c

@@ -31,6 +31,8 @@
 #include "celix_types.h"
 #include "celix_errno.h"
 #include "celix_build_assert.h"
+#include "celix_stdlib_cleanup.h"
+#include "celix_unistd_cleanup.h"
 #include <dns_sd.h>
 #include <netinet/in.h>
 #include <net/if.h>
@@ -89,39 +91,38 @@ static void *discoveryZeroconfAnnouncer_refreshEndpointThread(void *data);
 
 celix_status_t discoveryZeroconfAnnouncer_create(celix_bundle_context_t *ctx, celix_log_helper_t *logHelper, discovery_zeroconf_announcer_t **announcerOut) {
     celix_status_t status = CELIX_SUCCESS;
-    discovery_zeroconf_announcer_t *announcer = (discovery_zeroconf_announcer_t *)calloc(1, sizeof(*announcer));
+    celix_autofree discovery_zeroconf_announcer_t *announcer = (discovery_zeroconf_announcer_t *)calloc(1, sizeof(*announcer));
     if (announcer == NULL) {
         celix_logHelper_fatal(logHelper, "Announcer: Failed to alloc announcer.");
-        status = CELIX_ENOMEM;
-        goto announcer_err;
+        return CELIX_ENOMEM;
     }
     announcer->ctx = ctx;
     announcer->logHelper = logHelper;
     announcer->sharedRef = NULL;
 
     announcer->eventFd = eventfd(0, 0);
     if (announcer->eventFd < 0) {
-        status = CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO, errno);
         celix_logHelper_fatal(logHelper, "Announcer: Failed to open event fd, %d.", errno);
-        goto eventfd_err;
+        return CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO, errno);
     }
+    celix_auto(celix_fd_t) eventFd = announcer->eventFd;
 
     status = celixThreadMutex_create(&announcer->mutex, NULL);
     if (status != CELIX_SUCCESS) {
         celix_logHelper_fatal(logHelper, "Announcer: Failed to create mutex, %d.", status);
-        goto mutex_err;
+        return status;
     }
+    celix_autoptr(celix_thread_mutex_t) mutex = &announcer->mutex;
 
-    announcer->endpoints = celix_stringHashMap_create();
+    celix_autoptr(celix_string_hash_map_t) endpoints = announcer->endpoints = celix_stringHashMap_create();
     assert(announcer->endpoints != NULL);
-    announcer->revokedEndpoints = celix_arrayList_create();
+    celix_autoptr(celix_array_list_t) revokedEndpoints = announcer->revokedEndpoints = celix_arrayList_create();
     assert(announcer->revokedEndpoints != NULL);
 
     const char *fwUuid = celix_bundleContext_getProperty(ctx, OSGI_FRAMEWORK_FRAMEWORK_UUID, NULL);
     if (fwUuid == NULL || strlen(fwUuid) >= sizeof(announcer->fwUuid)) {
-        status = CELIX_BUNDLE_EXCEPTION;
         celix_logHelper_fatal(logHelper, "Announcer: Failed to get framework uuid.");
-        goto fw_uuid_err;
+        return CELIX_BUNDLE_EXCEPTION;
     }
     strcpy(announcer->fwUuid, fwUuid);
 
@@ -141,35 +142,29 @@ celix_status_t discoveryZeroconfAnnouncer_create(celix_bundle_context_t *ctx, ce
     opt.svc = &announcer->epListener;
     announcer->epListenerSvcId = celix_bundleContext_registerServiceWithOptionsAsync(ctx, &opt);
     if (announcer->epListenerSvcId < 0) {
-        status = CELIX_BUNDLE_EXCEPTION;
         celix_logHelper_fatal(logHelper, "Announcer: Failed to register endpoint listener.");
-        goto ep_listener_err;
+        return CELIX_BUNDLE_EXCEPTION;
     }
+    celix_auto(celix_service_reg_t) epListenerSvcReg = {
+        .ctx = ctx,
+        .svcId = announcer->epListenerSvcId
+    };
 
     announcer->running = true;
     status = celixThread_create(&announcer->refreshEPThread, NULL, discoveryZeroconfAnnouncer_refreshEndpointThread, announcer);
     if (status != CELIX_SUCCESS) {
         celix_logHelper_fatal(logHelper, "Announcer: Failed to create refreshing endpoint thread, %d.", status);
-        goto announce_ep_thread_err;
+        return status;
     }
     celixThread_setName(&announcer->refreshEPThread, "DiscAnnouncer");
 
-    *announcerOut = announcer;
+    epListenerSvcReg.svcId = -1;
+    celix_steal_ptr(revokedEndpoints);
+    celix_steal_ptr(endpoints);
+    celix_steal_ptr(mutex);
+    celix_steal_fd(&eventFd);
+    *announcerOut = celix_steal_ptr(announcer);
     return CELIX_SUCCESS;
-announce_ep_thread_err:
-    celix_bundleContext_unregisterServiceAsync(ctx, announcer->epListenerSvcId, NULL, NULL);
-    celix_bundleContext_waitForAsyncUnregistration(ctx, announcer->epListenerSvcId);
-ep_listener_err:
-fw_uuid_err:
-    celix_arrayList_destroy(announcer->revokedEndpoints);
-    celix_stringHashMap_destroy(announcer->endpoints);
-    celixThreadMutex_destroy(&announcer->mutex);
-mutex_err:
-    close(announcer->eventFd);
-eventfd_err:
-    free(announcer);
-announcer_err:
-    return status;
 }
 
 void discoveryZeroconfAnnouncer_destroy(discovery_zeroconf_announcer_t *announcer) {
@@ -229,7 +224,6 @@ static bool isLoopBackNetInterface(const char *ifName) {
 
 static  celix_status_t discoveryZeroconfAnnouncer_endpointAdded(void *handle, endpoint_description_t *endpoint, char *matchedFilter) {
     (void)matchedFilter;//unused
-    celix_status_t status = CELIX_SUCCESS;
     discovery_zeroconf_announcer_t *announcer = (discovery_zeroconf_announcer_t *)handle;
     assert(announcer != NULL);
     if (endpointDescription_isInvalid(endpoint)) {
@@ -239,11 +233,10 @@ static  celix_status_t discoveryZeroconfAnnouncer_endpointAdded(void *handle, en
 
     celix_logHelper_info(announcer->logHelper, "Announcer: Add endpoint for %s(%s).", endpoint->serviceName, endpoint->id);
 
-    announce_endpoint_entry_t *entry = (announce_endpoint_entry_t *)calloc(1, sizeof(*entry));
+    celix_autofree announce_endpoint_entry_t *entry = (announce_endpoint_entry_t *)calloc(1, sizeof(*entry));
     if (entry == NULL) {
         celix_logHelper_error(announcer->logHelper, "Announcer:  Failed to alloc endpoint entry.");
-        status = CELIX_ENOMEM;
-        goto endpoint_entry_err;
+        return CELIX_ENOMEM;
     }
     entry->registerRef = NULL;
     entry->announced = false;
@@ -275,35 +268,28 @@ static  celix_status_t discoveryZeroconfAnnouncer_endpointAdded(void *handle, en
         int bytes = snprintf(entry->serviceType, sizeof(entry->serviceType), DZC_SERVICE_PRIMARY_TYPE",%s", serviceSubType);
         if (bytes >= sizeof(entry->serviceType)) {
             celix_logHelper_error(announcer->logHelper, "Announcer: Please reduce the length of service type for %s.", serviceSubType);
-            status = CELIX_ILLEGAL_ARGUMENT;
-            goto service_type_err;
+            return CELIX_ILLEGAL_ARGUMENT;
         }
     } else {
         CELIX_BUILD_ASSERT(sizeof(entry->serviceType) >= sizeof(DZC_SERVICE_PRIMARY_TYPE));
         strcpy(entry->serviceType, DZC_SERVICE_PRIMARY_TYPE);
     }
-    entry->properties = celix_properties_copy(endpoint->properties);
+    celix_autoptr(celix_properties_t) properties = entry->properties = celix_properties_copy(endpoint->properties);
 
     //Remove properties that remote service does not need
     celix_properties_unset(entry->properties, CELIX_RSA_NETWORK_INTERFACES);
     celix_properties_unset(entry->properties, DZC_SERVICE_TYPE_KEY);
     entry->serviceName = celix_properties_get(entry->properties, OSGI_FRAMEWORK_OBJECTCLASS, NULL);
     if (entry->serviceName == NULL) {
-        status = CELIX_ILLEGAL_ARGUMENT;
         celix_logHelper_error(announcer->logHelper,"Announcer: Invalid service.");
-        goto service_name_err;
+        return CELIX_ILLEGAL_ARGUMENT;
     }
     celixThreadMutex_lock(&announcer->mutex);
-    celix_stringHashMap_put(announcer->endpoints, endpoint->id, entry);
+    celix_stringHashMap_put(announcer->endpoints, endpoint->id, celix_steal_ptr(entry));
     celixThreadMutex_unlock(&announcer->mutex);
     discoveryZeroconfAnnouncer_eventNotify(announcer);
+    celix_steal_ptr(properties);
     return CELIX_SUCCESS;
-service_name_err:
-    celix_properties_destroy(entry->properties);
-service_type_err:
-    free(entry);
-endpoint_entry_err:
-    return status;
 }
 
 static celix_status_t discoveryZeroconfAnnouncer_endpointRemoved(void *handle, endpoint_description_t *endpoint, char *matchedFilter) {

bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_announcer.h

@@ -23,6 +23,7 @@
 extern "C" {
 #endif
 
+#include "celix_cleanup.h"
 #include "celix_log_helper.h"
 #include "celix_types.h"
 #include "celix_errno.h"
@@ -33,6 +34,8 @@ celix_status_t discoveryZeroconfAnnouncer_create(celix_bundle_context_t *ctx, ce
 
 void discoveryZeroconfAnnouncer_destroy(discovery_zeroconf_announcer_t *announcer);
 
+CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(discovery_zeroconf_announcer_t, discoveryZeroconfAnnouncer_destroy)
+
 #ifdef __cplusplus
 }
 #endif

bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_watcher.c

@@ -25,7 +25,9 @@
 #include "celix_constants.h"
 #include "celix_long_hash_map.h"
 #include "celix_string_hash_map.h"
+#include "celix_stdlib_cleanup.h"
 #include "celix_threads.h"
+#include "celix_unistd_cleanup.h"
 #include "celix_utils.h"
 #include "celix_errno.h"
 #include <dns_sd.h>
@@ -89,45 +91,47 @@ static void *discoveryZeroconfWatcher_watchEPThread(void *data);
 
 celix_status_t discoveryZeroconfWatcher_create(celix_bundle_context_t *ctx, celix_log_helper_t *logHelper, discovery_zeroconf_watcher_t **watcherOut) {
     celix_status_t status = CELIX_SUCCESS;
-    discovery_zeroconf_watcher_t *watcher = (discovery_zeroconf_watcher_t *)calloc(1, sizeof(*watcher));
+    celix_autofree discovery_zeroconf_watcher_t *watcher = (discovery_zeroconf_watcher_t *)calloc(1, sizeof(*watcher));
     if (watcher == NULL) {
         celix_logHelper_fatal(logHelper, "Watcher: Failed to alloc watcher.");
-        status = CELIX_ENOMEM;
-        goto watcher_err;
+        return CELIX_ENOMEM;
     }
     watcher->logHelper = logHelper;
     watcher->ctx = ctx;
     watcher->sharedRef = NULL;
     watcher->browseRef = NULL;
     watcher->eventFd = eventfd(0, 0);
     if (watcher->eventFd < 0) {
-        status = CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO, errno);
         celix_logHelper_fatal(logHelper, "Watcher: Failed to open event fd, %d.", errno);
-        goto event_fd_err;
+        return CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO, errno);
     }
+    celix_auto(celix_fd_t) eventFd = watcher->eventFd;
 
     const char *fwUuid = celix_bundleContext_getProperty(ctx, OSGI_FRAMEWORK_FRAMEWORK_UUID, NULL);
     if (fwUuid == NULL || strlen(fwUuid) >= sizeof(watcher->fwUuid)) {
-        status = CELIX_BUNDLE_EXCEPTION;
         celix_logHelper_fatal(logHelper, "Watcher: Failed to get framework uuid.");
-        goto fw_uuid_err;
+        return CELIX_BUNDLE_EXCEPTION;
     }
     strcpy(watcher->fwUuid, fwUuid);
 
     status = celixThreadMutex_create(&watcher->mutex, NULL);
     if (status != CELIX_SUCCESS) {
         celix_logHelper_fatal(logHelper, "Watcher: Failed to create mutex, %d.", status);
-        goto mutex_err;
+        return status;
     }
+    celix_autoptr(celix_thread_mutex_t) mutex = &watcher->mutex;
     celix_string_hash_map_create_options_t epOpts = CELIX_EMPTY_STRING_HASH_MAP_CREATE_OPTIONS;
     epOpts.storeKeysWeakly = true;
-    watcher->watchedEndpoints = celix_stringHashMap_createWithOptions(&epOpts);
+    celix_autoptr(celix_string_hash_map_t) watchedEndpoints =
+        watcher->watchedEndpoints = celix_stringHashMap_createWithOptions(&epOpts);
     assert(watcher->watchedEndpoints);
     celix_string_hash_map_create_options_t svcOpts = CELIX_EMPTY_STRING_HASH_MAP_CREATE_OPTIONS;
-    watcher->watchedServices = celix_stringHashMap_createWithOptions(&svcOpts);
+    celix_autoptr(celix_string_hash_map_t) watchedServices =
+        watcher->watchedServices = celix_stringHashMap_createWithOptions(&svcOpts);
     assert(watcher->watchedServices != NULL);
 
-    watcher->epls = celix_longHashMap_create();
+    celix_autoptr(celix_long_hash_map_t) epls =
+        watcher->epls = celix_longHashMap_create();
     assert(watcher->epls != NULL);
 
     celix_service_tracking_options_t opts = CELIX_EMPTY_SERVICE_TRACKING_OPTIONS;
@@ -138,35 +142,26 @@ celix_status_t discoveryZeroconfWatcher_create(celix_bundle_context_t *ctx, celi
     opts.removeWithProperties = discoveryZeroconfWatcher_removeEPL;
     watcher->epListenerTrkId = celix_bundleContext_trackServicesWithOptionsAsync(ctx, &opts);
     if (watcher->epListenerTrkId < 0) {
-        status = CELIX_BUNDLE_EXCEPTION;
         celix_logHelper_fatal(logHelper, "Watcher: Failed to register endpoint listener service.");
-        goto epl_tracker_err;
+        return CELIX_BUNDLE_EXCEPTION;
     }
 
     watcher->running = true;
     status = celixThread_create(&watcher->watchEPThread,NULL, discoveryZeroconfWatcher_watchEPThread, watcher);
     if (status != CELIX_SUCCESS) {
-        goto thread_err;
+        celix_bundleContext_stopTrackerAsync(ctx, watcher->epListenerTrkId, NULL, NULL);
+        celix_bundleContext_waitForAsyncStopTracker(ctx, watcher->epListenerTrkId);
+        return status;
     }
     celixThread_setName(&watcher->watchEPThread, "DiscWatcher");
 
-    *watcherOut = watcher;
+    celix_steal_ptr(epls);
+    celix_steal_ptr(watchedServices);
+    celix_steal_ptr(watchedEndpoints);
+    celix_steal_ptr(mutex);
+    celix_steal_fd(&eventFd);
+    *watcherOut = celix_steal_ptr(watcher);
     return CELIX_SUCCESS;
-thread_err:
-    celix_bundleContext_stopTrackerAsync(ctx, watcher->epListenerTrkId, NULL, NULL);
-    celix_bundleContext_waitForAsyncStopTracker(ctx, watcher->epListenerTrkId);
-epl_tracker_err:
-    celix_longHashMap_destroy(watcher->epls);
-    celix_stringHashMap_destroy(watcher->watchedServices);
-    celix_stringHashMap_destroy(watcher->watchedEndpoints);
-    celixThreadMutex_destroy(&watcher->mutex);
-mutex_err:
-fw_uuid_err:
-    close(watcher->eventFd);
-event_fd_err:
-    free(watcher);
-watcher_err:
-    return status;
 }
 
 void discoveryZeroconfWatcher_destroy(discovery_zeroconf_watcher_t *watcher) {

bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_watcher.h

@@ -23,6 +23,7 @@
 extern "C" {
 #endif
 
+#include "celix_cleanup.h"
 #include "celix_log_helper.h"
 #include "celix_types.h"
 #include "celix_errno.h"
@@ -33,6 +34,8 @@ celix_status_t discoveryZeroconfWatcher_create(celix_bundle_context_t *ctx, celi
 
 void discoveryZeroconfWatcher_destroy(discovery_zeroconf_watcher_t *watcher);
 
+CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(discovery_zeroconf_watcher_t, discoveryZeroconfWatcher_destroy)
+
 #ifdef __cplusplus
 }
 #endif