celix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bjoern Petri <bjoern.pe...@sundevil.de>
Subject Re: svn commit: r1628887 - in /celix/trunk/remote_services/remote_service_admin_http/private: include/remote_service_admin_http_impl.h src/remote_service_admin_impl.c
Date Thu, 02 Oct 2014 06:57:55 GMT
On 2014-10-02 08:33, Alexander Broekhuis wrote:
> Looking at the commit, I do have some remarks on the code.
> 
> The for loop for getting the address does not break if it finds a 
> match. So
> if no interface is given, it will use the last one found, instead of 
> the
> first one. And if a interface is given, it will continue checking the
> others, even if a match is found.

fixed.


> Also the getIpAddress does not follow the default function structure, 
> ie
> the first argument is not the RSA. I see that it is not needed in this
> case, so I am not sure what is cleaner/nicer to do. Since it is a 
> static
> (private) function, this could be accepted. What do you think?

Well, the getIpAddress function is a quite common function which is only 
located within the remote_service_admin_impl.c because it is only used 
there. If we could re-use it in another bundle as well I would actually 
consider putting it at some common code part (Without any bundle 
dependency). Hence, in my opinion it is acceptable to have it without 
the RSA argument.

> Finally, there is some debug printing left in the function that can be
> cleaned up.

Well, that's what happens if you do last-minute-commits before you have 
to leave the train ;)


> What this does not solve is how to use the RSA on multiple address, but 
> I
> guess that requires a bit more discussion/work.

I also think this will not work for IPv6. But I would also like to 
postpone this a little bit as well.


> 2014-10-02 7:25 GMT+02:00 <bpetri@apache.org>:
> 
>> Author: bpetri
>> Date: Thu Oct  2 05:25:46 2014
>> New Revision: 1628887
>> 
>> URL: http://svn.apache.org/r1628887
>> Log:
>> CELIX-160: replaced apr code
>> 
>> Modified:
>> 
>> celix/trunk/remote_services/remote_service_admin_http/private/include/remote_service_admin_http_impl.h
>> 
>> celix/trunk/remote_services/remote_service_admin_http/private/src/remote_service_admin_impl.c
>> 
>> Modified:
>> celix/trunk/remote_services/remote_service_admin_http/private/include/remote_service_admin_http_impl.h
>> URL:
>> http://svn.apache.org/viewvc/celix/trunk/remote_services/remote_service_admin_http/private/include/remote_service_admin_http_impl.h?rev=1628887&r1=1628886&r2=1628887&view=diff
>> 
>> ==============================================================================
>> ---
>> celix/trunk/remote_services/remote_service_admin_http/private/include/remote_service_admin_http_impl.h
>> (original)
>> +++
>> celix/trunk/remote_services/remote_service_admin_http/private/include/remote_service_admin_http_impl.h
>> Thu Oct  2 05:25:46 2014
>> @@ -38,6 +38,7 @@ struct remote_service_admin {
>>         hash_map_pt importedServices;
>> 
>>         char *port;
>> +       char *ip;
>> 
>>         struct mg_context *ctx;
>>  };
>> 
>> Modified:
>> celix/trunk/remote_services/remote_service_admin_http/private/src/remote_service_admin_impl.c
>> URL:
>> http://svn.apache.org/viewvc/celix/trunk/remote_services/remote_service_admin_http/private/src/remote_service_admin_impl.c?rev=1628887&r1=1628886&r2=1628887&view=diff
>> 
>> ==============================================================================
>> ---
>> celix/trunk/remote_services/remote_service_admin_http/private/src/remote_service_admin_impl.c
>> (original)
>> +++
>> celix/trunk/remote_services/remote_service_admin_http/private/src/remote_service_admin_impl.c
>> Thu Oct  2 05:25:46 2014
>> @@ -26,6 +26,14 @@
>>  #include <stdio.h>
>>  #include <stdlib.h>
>> 
>> +#include <arpa/inet.h>
>> +#include <sys/socket.h>
>> +#include <netdb.h>
>> +#include <ifaddrs.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +#include <string.h>
>>  #include <apr_strings.h>
>>  #include <apr_uuid.h>
>>  #include <uuid/uuid.h>
>> @@ -71,7 +79,8 @@ static int remoteServiceAdmin_callback(s
>> 
>>  celix_status_t 
>> remoteServiceAdmin_installEndpoint(remote_service_admin_pt
>> admin, export_registration_pt registration, service_reference_pt 
>> reference,
>> char *interface);
>>  celix_status_t
>> remoteServiceAdmin_createEndpointDescription(remote_service_admin_pt 
>> admin,
>> service_reference_pt reference, properties_pt endpointProperties, char
>> *interface, endpoint_description_pt *description);
>> -static celix_status_t constructServiceUrl(remote_service_admin_pt 
>> admin,
>> char *service, char **serviceUrl);
>> +
>> +static celix_status_t remoteServiceAdmin_getIpAdress(char* interface,
>> char** ip);
>> 
>>  static size_t remoteServiceAdmin_readCallback(void *ptr, size_t size,
>> size_t nmemb, void *userp);
>>  static size_t remoteServiceAdmin_write(void *contents, size_t size,
>> size_t nmemb, void *userp);
>> @@ -90,12 +99,38 @@ celix_status_t remoteServiceAdmin_create
>> 
>>                 // Start webserver
>>                 char *port = NULL;
>> +               char *ip = NULL;
>> +
>>                 bundleContext_getProperty(context, "RSA_PORT", &port);
>>                 if (port == NULL) {
>>                         (*admin)->port = (char *)DEFAULT_PORT;
>>                 } else {
>>                         (*admin)->port = apr_pstrdup(pool, port);
>>                 }
>> +
>> +               bundleContext_getProperty(context, "RSA_IP", &ip);
>> +               if (ip == NULL) {
>> +                       char *interface = NULL;
>> +
>> +                       bundleContext_getProperty(context,
>> "RSA_INTERFACE", &interface);
>> +                       if ((interface != NULL) &&
>> (remoteServiceAdmin_getIpAdress(interface, &ip) != CELIX_SUCCESS)) {
>> +                               fw_log(logger, 
>> OSGI_FRAMEWORK_LOG_WARNING,
>> "RSA: Could not retrieve IP adress for interface %s\n", interface);
>> +                       }
>> +
>> +                       if (ip == NULL) {
>> +                               remoteServiceAdmin_getIpAdress(NULL, 
>> &ip);
>> +                       }
>> +               }
>> +
>> +               if (ip != NULL) {
>> +                       fw_log(logger, OSGI_FRAMEWORK_LOG_INFO, "RSA:
>> Using %s for service annunciation", ip);
>> +                       (*admin)->ip = apr_pstrdup(pool, ip);
>> +               }
>> +               else {
>> +                       fw_log(logger, OSGI_FRAMEWORK_LOG_ERROR, "RSA: 
>> No
>> IP address for service annunciation set.");
>> +               }
>> +
>> +
>>                 fw_log(logger, OSGI_FRAMEWORK_LOG_INFO, "RSA: Start
>> webserver: %s", (*admin)->port);
>>                 const char *options[] = { "listening_ports",
>> (*admin)->port, NULL};
>> 
>> @@ -337,8 +372,7 @@ celix_status_t remoteServiceAdmin_instal
>> 
>>         char buf[512];
>>         sprintf(buf, "/service/%s/%s", serviceId, interface);
>> -    char *url = NULL;
>> -    constructServiceUrl(admin,buf, &url);
>> +    char *url = apr_pstrcat(admin->pool, "http://", admin->ip, ":",
>> admin->port, buf, NULL);
>> 
>>         uuid_t endpoint_uid;
>>         uuid_generate(endpoint_uid);
>> @@ -361,44 +395,42 @@ celix_status_t remoteServiceAdmin_instal
>>         return status;
>>  }
>> 
>> -static celix_status_t constructServiceUrl(remote_service_admin_pt 
>> admin,
>> char *service, char **serviceUrl) {
>> -       celix_status_t status = CELIX_SUCCESS;
>> -
>> -       if (*serviceUrl != NULL || admin == NULL || service == NULL ) 
>> {
>> -               status = CELIX_ILLEGAL_ARGUMENT;
>> -       } else {
>> -               char host[APRMAXHOSTLEN + 1];
>> -               apr_sockaddr_t *sa;
>> -               char *ip = NULL;
>> -
>> -               bundleContext_getProperty(admin->context, "RSA_IP", 
>> &ip);
>> +static celix_status_t remoteServiceAdmin_getIpAdress(char* interface,
>> char** ip) {
>> +       celix_status_t status = CELIX_BUNDLE_EXCEPTION;
>> 
>> -               if (ip == NULL) {
>> -                       apr_status_t stat = apr_gethostname(host,
>> APRMAXHOSTLEN + 1, admin->pool); /*TODO mem leak*/
>> -                       if (stat != APR_SUCCESS) {
>> -                               status = CELIX_BUNDLE_EXCEPTION;
>> -                       } else {
>> -                               stat = apr_sockaddr_info_get(&sa, 
>> host,
>> APR_INET, 0, 0, admin->pool); /*TODO mem leak*/
>> -                               if (stat != APR_SUCCESS) {
>> -                                       status = 
>> CELIX_BUNDLE_EXCEPTION;
>> -                               } else {
>> -                                       stat = 
>> apr_sockaddr_ip_get(&ip,
>> sa);
>> -                                       if (stat != APR_SUCCESS) {
>> -                                               status =
>> CELIX_BUNDLE_EXCEPTION;
>> -                                       } else {
>> -                                               *serviceUrl =
>> apr_pstrcat(admin->pool, "http://", ip, ":", admin->port, service, 
>> NULL );
>> -                                       }
>> +       struct ifaddrs *ifaddr, *ifa;
>> +    char host[NI_MAXHOST];
>> +
>> +    if (getifaddrs(&ifaddr) != -1)
>> +    {
>> +               for (ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next)
>> +               {
>> +                       if (ifa->ifa_addr == NULL)
>> +                               continue;
>> +
>> +                       printf("ITERATE\n");
>> +
>> +                       if ((getnameinfo(ifa->ifa_addr,sizeof(struct
>> sockaddr_in), host, NI_MAXHOST, NULL, 0, NI_NUMERICHOST) == 0) &&
>> (ifa->ifa_addr->sa_family == AF_INET)) {
>> +                            printf("\tInterface : 
>> <%s>\n",ifa->ifa_name );
>> +                                   printf("\t  Address : <%s>\n", 
>> host);
>> +                               if (interface == NULL) {
>> +                                       *ip = strdup(host);
>> +                                       status = CELIX_SUCCESS;
>> +                               }
>> +                               else if (strcmp(ifa->ifa_name, 
>> interface)
>> == 0) {
>> +                                       *ip = strdup(host);
>> +                                       status = CELIX_SUCCESS;
>>                                 }
>>                         }
>>                 }
>> -               else {
>> -                       *serviceUrl = apr_pstrcat(admin->pool, 
>> "http://",
>> ip, ":", admin->port, service, NULL );
>> -               }
>> -       }
>> -
>> -       return status;
>> +
>> +               freeifaddrs(ifaddr);
>> +    }
>> +
>> +    return status;
>>  }
>> 
>> +
>>  celix_status_t
>> remoteServiceAdmin_createEndpointDescription(remote_service_admin_pt 
>> admin,
>> service_reference_pt reference,
>>                 properties_pt endpointProperties, char *interface,
>> endpoint_description_pt *description) {
>>         celix_status_t status = CELIX_SUCCESS;
>> 
>> 
>> 

Mime
View raw message