PengZheng commented on code in PR #710: URL: https://github.com/apache/celix/pull/710#discussion_r1483889023
########## bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c: ########## @@ -952,7 +966,80 @@ celix_status_t remoteServiceAdmin_removeImportedService(remote_service_admin_t * return status; } -static celix_status_t remoteServiceAdmin_send(void *handle, endpoint_description_t *endpointDescription, char *request, celix_properties_t *metadata, char **reply, int* replyStatus) { +static celix_status_t remoteServiceAdmin_useDynamicIpUrlsForEndpoint(remote_service_admin_t* rsa, endpoint_description_t* endpointDescription, +celix_status_t (*useUrl)(void* handle, const char* url), void* handle) { + celix_status_t status = CELIX_ERROR_MAKE(CELIX_FACILITY_CURL,CURLE_COULDNT_CONNECT); + celixThreadMutex_lock(&rsa->importedServicesLock); + celix_imported_endpoint_url_ref_t* urlRef = celix_stringHashMap_get(rsa->importedEndpointUrls, endpointDescription->id); + if (urlRef != NULL) { + celix_ref_get(&urlRef->ref); + celixThreadMutex_unlock(&rsa->importedServicesLock); + status = useUrl(handle, urlRef->url); + celix_ref_put(&urlRef->ref, (void*)free); + } else { + celixThreadMutex_unlock(&rsa->importedServicesLock); + } + if (status != CELIX_ERROR_MAKE(CELIX_FACILITY_CURL,CURLE_COULDNT_CONNECT)) { Review Comment: Is live lock possible here? That is, we are stuck with a URL that curl keeps returning an error other than `CURLE_COULDNT_CONNECT`. ########## bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c: ########## @@ -952,7 +966,80 @@ celix_status_t remoteServiceAdmin_removeImportedService(remote_service_admin_t * return status; } -static celix_status_t remoteServiceAdmin_send(void *handle, endpoint_description_t *endpointDescription, char *request, celix_properties_t *metadata, char **reply, int* replyStatus) { +static celix_status_t remoteServiceAdmin_useDynamicIpUrlsForEndpoint(remote_service_admin_t* rsa, endpoint_description_t* endpointDescription, +celix_status_t (*useUrl)(void* handle, const char* url), void* handle) { + celix_status_t status = CELIX_ERROR_MAKE(CELIX_FACILITY_CURL,CURLE_COULDNT_CONNECT); + celixThreadMutex_lock(&rsa->importedServicesLock); + celix_imported_endpoint_url_ref_t* urlRef = celix_stringHashMap_get(rsa->importedEndpointUrls, endpointDescription->id); + if (urlRef != NULL) { + celix_ref_get(&urlRef->ref); + celixThreadMutex_unlock(&rsa->importedServicesLock); + status = useUrl(handle, urlRef->url); + celix_ref_put(&urlRef->ref, (void*)free); + } else { + celixThreadMutex_unlock(&rsa->importedServicesLock); + } + if (status != CELIX_ERROR_MAKE(CELIX_FACILITY_CURL,CURLE_COULDNT_CONNECT)) { + return status; + } + + const char *serviceUrl = celix_properties_get(endpointDescription->properties, (char*) RSA_DFI_ENDPOINT_URL, ""); + const char *ipaddresses = celix_properties_get(endpointDescription->properties, CELIX_RSA_IP_ADDRESSES, NULL); + int port = (int)celix_properties_getAsLong(endpointDescription->properties, CELIX_RSA_PORT, RSA_PORT_DEFAULT); + celix_autofree char *ipStrListCopy = NULL; + ipStrListCopy = celix_utils_strdup(ipaddresses); + if (ipStrListCopy == NULL) { + celix_logHelper_error(rsa->loghelper, "Error duplicating ip addresses"); + return CELIX_ENOMEM; + } + char url[256] = {0}; + char *savePtr = NULL; + char *token = strtok_r(ipStrListCopy, ",", &savePtr); + while (token != NULL) { + char *ip = celix_utils_trimInPlace(token); + struct sockaddr_in sa; + if (inet_pton(AF_INET, ip, &(sa.sin_addr)) == 1) { + snprintf(url, sizeof(url), "http://%s:%d%s", ip, port, serviceUrl); + } else { + snprintf(url, sizeof(url), "http://[%s]:%d%s", ip, port, serviceUrl); + } + status = useUrl(handle, url); + if (status != CELIX_ERROR_MAKE(CELIX_FACILITY_CURL,CURLE_COULDNT_CONNECT)) { Review Comment: I found it dubious to accept any URL that CURL returns an error other than `CURLE_COULDNT_CONNECT`. -- 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