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

[Luca Abeni]

Hi,

On 04/01/10 00:11, Martin Storsj? wrote:
> Hi,
>
> As discussed with Ronald a few days ago, this is a first shot at moving
> most usage of host resolving functions to use getaddrinfo instead of
> gethostbyname. It isn't tested all that much yet, though.

As requested, here are some comments on patch 0008:
diff --git a/libavformat/udp.c b/libavformat/udp.c
index a89de00..4f124c2 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -240,13 +240,27 @@ static int udp_port(struct sockaddr_storage *addr, 
int addr_len)

  static int udp_set_url(struct sockaddr_in *addr, const char *hostname, 
int port)
  {
-    /* set the destination address */
-    if (resolve_host(&addr->sin_addr, hostname) < 0)
+    struct addrinfo hints, *ai, *cur;
+    char portstr[10];
[...]
Here, it seems to me that you are duplicating the code used by the 
version of udp_port() used if CONFIG_IPV6 is defined (the code you are 
adding looks very much like the udp_ipv6_resolve_host() function.

Since noone seems to like the current approach (having two 
implementations of udp_set_url() and similar functions - one is used 
when getaddrinfo() is available, the other one when it is not), 
switching to a different approach is a good idea. But this patch risks 
to create a lot of code duplication... Maybe just removing the "#if 
CONFIG_IPV6" and using one single udp_set_url() implementation is a 
better idea.

I am surely missing something obvious, but why not just always defining 
CONFIG_IPV6 (removing the code used when it is not defined)? 
(Provinding, of course, a fallback getaddrinfo() implementation if the 
system does not provide it)

[...]
+    for (cur = ai; cur; cur = cur->ai_next) {
+        if (cur->ai_family == AF_INET) {
+            *addr = *(struct sockaddr_in *)cur->ai_addr;
+            freeaddrinfo(ai);
+            return sizeof(struct sockaddr_in);
+        }
+    }

This for() loop always confused me...


				Luca