celix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Broekhuis <a.broekh...@gmail.com>
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:33:21 GMT
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.

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?

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

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


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;
>
>
>


-- 
Met vriendelijke groet,

Alexander Broekhuis

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message