qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alan Conway <acon...@redhat.com>
Subject Re: Question on preferences for platform diffs in C++ code
Date Tue, 25 Mar 2008 16:06:40 GMT
Steve Huston wrote:
> Hi,
> 
> I'm working on porting the qpid C++ code to Windows. I've got changes
> for 1 file to get it to build, cpp/src/qpid/Url.cpp. This required
> Windows-specific code:
> 
> Index: Url.cpp
> ===================================================================
> --- Url.cpp     (revision 638488)
> +++ Url.cpp     (working copy)
> @@ -24,10 +24,17 @@
>  #include <boost/spirit.hpp>
>  #include <boost/spirit/actor.hpp>
> 
> -#include <sys/ioctl.h>
> -#include <net/if.h>
> -#include <unistd.h>
> -#include <arpa/inet.h>
> +#ifdef _WINDOWS
> +#  include <ws2tcpip.h>
> +#  ifndef HOST_NAME_MAX
> +#    define HOST_NAME_MAX 256
> +#  endif
> +#else
> +#  include <sys/ioctl.h>
> +#  include <net/if.h>
> +#  include <unistd.h>
> +#  include <arpa/inet.h>
> +#endif /* _WINDOWS */
>  #include <stdio.h>
>  #include <errno.h>
> 
> @@ -53,6 +60,33 @@
> 
>  Url Url::getIpAddressesUrl(uint16_t port) {
>      Url url;
> +
> +#ifdef _WINDOWS
> +    enum { MAX_URL_INTERFACES = 100 };
> +    SOCKET s = socket (PF_INET, SOCK_STREAM, 0);
> +    if (s != INVALID_SOCKET) {
> +        INTERFACE_INFO interfaces[MAX_URL_INTERFACES];
> +        DWORD filledBytes = 0;
> +        WSAIoctl (s,
> +                  SIO_GET_INTERFACE_LIST,
> +                  0,
> +                  0,
> +                  interfaces,
> +                  sizeof (interfaces),
> +                  &filledBytes,
> +                  0,
> +                  0);
> +        unsigned int interfaceCount = filledBytes / sizeof
> (INTERFACE_INFO);
> +        for (unsigned int i = 0; i < interfaceCount; ++i) {
> +            if (interfaces[i].iiFlags & IFF_UP) {
> +                string
> addr(inet_ntoa(interfaces[i].iiAddress.AddressIn.sin_add
> r));
> +                if (addr != LOCALHOST)
> +                    url.push_back(TcpAddress(addr, port));
> +            }
> +        }
> +        closesocket (s);
> +    }
> +#else
>      int s = socket (PF_INET, SOCK_STREAM, 0);
>      for (int i=1;;i++) {
>          struct ifreq ifr;
> @@ -68,6 +102,8 @@
>              url.push_back(TcpAddress(addr, port));
>      }
>      close (s);
> +#endif /* _WINDOWS */
> +
>      return url;
>  }
> 
> I'm continuing on with compile errors and the next one is from
> cpp/src/qpid/sys/SystemInfo.cpp. It has a sysconf() call with a
> comment that the sysconf value is Linux-specific. This leads me to two
> questions for you all:
> 
> 1. Is the above diff along the lines of what's acceptable? Any major
> snafus I should fix before continuing? When this type of thing comes
> up, do you prefer it be done with conditional compiles like this, or
> refactor things so they're easily split? For example, create a
> class/function that returns a vector of strings with ip addresses, and
> have Url call it then build the url.

I prefer the latter - separate .cpp files for stuff that's platform dependent.


> 2. Some stuff, such as the SystemInfo.cpp sysconf() call, could be set
> up with autoconf-discovered information. Although autoconf is used,
> was/is it your intention to take care of all the platform specifics
> with it? This won't help with Windows, but I imagine it will help if
> there are other configurable ports attempted in the future.

This is only needed for the broker, you could move it to the broker lib so  you 
can do a client-only build without tripping over it. The SystemInfo class name 
is overly general, I thought at the time it might be a placeholder for other 
system stuff but we haven't come up with any so far...



Mime
View raw message