[FFmpeg-devel] [PATCH/RFC] Prefer getaddrinfo over gethostbyname

[Måns Rullgård]

Martin Storsj? <martin at martin.st> writes:

> On Tue, 12 Jan 2010, Ronald S. Bultje wrote:
>
>> On Tue, Jan 12, 2010 at 2:17 PM, Dave Yeo <daveryeo at telus.net> wrote:
>> > FYI sa_len is implemented like this here (the TCPV40HDRS are depreciated) on
>> > OS/2
>> >
>> > struct sockaddr {
>> > #ifdef TCPV40HDRS
>> > ? ? ? ?u_short sa_family; ? ? ? ? ? ? ?/* address family */
>> > ? ? ? ?char ? ?sa_data[14]; ? ? ? ? ? ?/* up to 14 bytes of direct address
>> > */
>> > #else
>> > ? ? ? ?u_char ?sa_len; ? ? ? ? ? ? ? ? /* total length */
>> > ? ? ? ?u_char ?sa_family; ? ? ? ? ? ? ?/* address family */
>> > ? ? ? ?char ? ?sa_data[14]; ? ? ? ? ? ?/* actually longer; address value */
>> > #endif
>> > };
>> 
>> could you implement a configure check and the above, which works?
>
> What about the attached patches? They use the newly added check_struct in 
> configure, to check for the sa_len field, and then provide a RFC 3493 
> compliant struct sockaddr_storage based on this info.
>
> This version can of course break, too, if some platform has some really 
> weird version of sockaddr, but it should at least handle these both 
> variations.
>
> // Martin
>
> From 9cd05f3d8f3cdbca001e733abe70642fa2edf083 Mon Sep 17 00:00:00 2001
> From: Martin Storsjo <martin at martin.st>
> Date: Wed, 20 Jan 2010 12:47:19 +0200
> Subject: [PATCH 1/2] Check for the sa_len member in struct sockaddr
>
> ---
>  configure |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/configure b/configure
> index f9287a9..0c28526 100755
> --- a/configure
> +++ b/configure
> @@ -1025,6 +1025,7 @@ HAVE_LIST="
>      posix_memalign
>      round
>      roundf
> +    sa_len
>      sdl
>      sdl_video_size
>      setmode
> @@ -2502,6 +2503,7 @@ if enabled network; then
>      check_type "sys/types.h sys/socket.h" socklen_t
>      check_type netdb.h "struct addrinfo"
>      check_type "sys/types.h sys/socket.h" "struct sockaddr_storage"
> +    check_struct "sys/types.h sys/socket.h" "struct sockaddr" sa_len && enable sa_len
>      # Prefer arpa/inet.h over winsock2
>      if check_header arpa/inet.h ; then
>          check_func closesocket
> @@ -2513,6 +2515,7 @@ if enabled network; then
>          check_type ws2tcpip.h socklen_t
>          check_type ws2tcpip.h "struct addrinfo"
>          check_type ws2tcpip.h "struct sockaddr_storage"
> +        check_struct winsock2.h "struct sockaddr" sa_len && enable sa_len
>      else
>          disable network
>      fi

Why the extra enable?  check_struct sets struct_sockddr_sa_len for you.

> From 6f378174a22152431158286915cc1f043d4402b9 Mon Sep 17 00:00:00 2001
> From: Martin Storsjo <martin at martin.st>
> Date: Wed, 20 Jan 2010 12:54:38 +0200
> Subject: [PATCH 2/2] Add proper fields to the struct sockaddr_storage replacement
>
> These fields follow the recommendation in RFC 3493. The only case
> they'd break is if the sa_family/sa_len fields of struct sockaddr
> are located somewhere else than immediately at the beginning of the
> struct, or if their combined size isn't 2 bytes.
> ---
>  libavformat/network.h |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/libavformat/network.h b/libavformat/network.h
> index ea8e946..15e3e70 100644
> --- a/libavformat/network.h
> +++ b/libavformat/network.h
> @@ -70,7 +70,15 @@ int inet_aton (const char * str, struct in_addr * add);
>  
>  #if !HAVE_STRUCT_SOCKADDR_STORAGE
>  struct sockaddr_storage {
> -    struct sockaddr_in x;
> +#if HAVE_SA_LEN
> +    uint8_t ss_len;
> +    uint8_t ss_family;
> +#else
> +    uint16_t ss_family;
> +#endif
> +    char __ss_pad1[6];
> +    int64_t __ss_align;
> +    char __ss_pad2[112];
>  };
>  #endif

You can't have names starting with __.

Why don't you use the system struct sockaddr instead of trying to
replicate it?  If it's not big enough for ipv6 addresses, the system
clearly doesn't support ipv6 in the first place.

-- 
M?ns Rullg?rd
mans at mansr.com