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

Reply via email to