[FFmpeg-devel] [PATCH] ipv6-compatible resolve_host() replacement

[Michael Niedermayer]

Hi

On Sat, Sep 29, 2007 at 12:37:18PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On 9/29/07, Michael Niedermayer <michaelni at gmx.at> wrote:
> >
> > i dont mind where it goes as long as its not a public header of
> > libavformat
> > and non public headers can be renamed at will anyway so the name is not
> > important, network.h resolve.h anything you like iam fine with it
> 
> 
> Ok, network.h it is then. New patch attached.



[...]

> +    for (res = resl, addri = addr; res; addri = addri->next, res = res->ai_next) {

there are quite a lot of statements on a single line, i think it would
be more readable if there where fewer



> +        addri->next = res->ai_next ? &addri[1] : NULL;
> +        addri->protocol = res->ai_protocol;
> +        addri->family = res->ai_family;
> +        addri->socket_type = res->ai_socktype;
> +        addri->addr_len = res->ai_addrlen;
> +        addri->addr = (struct sockaddr *) ptr;

these can be vertically aligned



[...]

> +void inetaddr_setport(AVInetAddr *addr, int port)
> +{
> +#ifdef CONFIG_IPV6
> +    if (addr->family == AF_INET6)
> +        ((struct sockaddr_in6 *) addr->addr)->sin6_port = htons(port);
> +    else /* AF_INET */
> +#endif
> +        ((struct sockaddr_in *) addr->addr)->sin_port = htons(port);
> +
> +    addr->port = port;
> +}
> +
> +AVInetAddr *inetaddr_alloc(int socket_stream)
> +{
> +    AVInetAddr *addr;
> +    int extra_size;
> +#ifdef CONFIG_IPV6
> +    extra_size = sizeof(struct sockaddr_in6);
> +#else
> +    extra_size = sizeof(struct sockaddr_in);
> +#endif
> +    addr = av_malloc(sizeof(AVInetAddr) + extra_size);
> +    addr->addr = (void *) &addr[1];
> +    addr->addr_len = extra_size;
> +    addr->next = NULL;
> +    addr->family = addr->port = addr->protocol = 0;
> +    addr->socket_type = socket_stream ? SOCK_STREAM : SOCK_DGRAM;
> +
> +    return addr;
> +}
> +
> +void inetaddr_reparse(AVInetAddr *addr)
> +{
> +    addr->family = addr->addr->sa_family;
> +#ifdef CONFIG_IPV6
> +    if (addr->family == AF_INET6)
> +        addr->port = ntohs(((struct sockaddr_in6 *) addr->addr)->sin6_port);
> +    else
> +#endif
> +        addr->port = ntohs(((struct sockaddr_in *) addr->addr)->sin_port);
> +}

these functions are unused
why are they here?


> +

> +char *inetaddr_str(AVInetAddr *addr, char *buf, int len)
> +{
> +#ifdef CONFIG_IPV6
> +    void *src_ptr;
> +
> +    if (addr->family == AF_INET) {
> +        if (((struct sockaddr_in *) addr->addr)->sin_addr.s_addr == INADDR_ANY) {
> +            av_strlcpy(buf, "127.0.0.1", len);
> +            return buf;
> +        }
> +        src_ptr = &((struct sockaddr_in *) addr->addr)->sin_addr;
> +    } else if (addr->family == AF_INET6) {
> +        static const struct in6_addr any_addr = IN6ADDR_ANY_INIT;
> +        if (memcmp (&((struct sockaddr_in6 *) addr->addr)->sin6_addr,
> +                    &any_addr, sizeof(any_addr))) {
> +            av_strlcpy(buf, "::1", len);
> +            return buf;
> +        }
> +        src_ptr = &((struct sockaddr_in6 *) addr->addr)->sin6_addr;
> +    } else
> +        return NULL;
> +    if (!inet_ntop(addr->family, src_ptr, buf, len))
> +        return NULL;
> +#else

> +    const char *res;
> +
> +    if (addr->family != AF_INET)
> +      return NULL;
> +    else if (((struct sockaddr_in *) addr->addr)->sin_addr.s_addr == INADDR_ANY) {

please declare this with the proper struct, if needed with a union or
whatever, these casts all over the code are ugly


> +        av_strlcpy(buf, "127.0.0.1", len);
> +        return buf;

duplicate


> +    } else if (!(res = inet_ntoa(((struct sockaddr_in *) addr->addr)->sin_addr)))
> +        return NULL;

docs say:
     Instead of `inet_ntoa' the newer function `inet_ntop' which is
     described below should be used since it handles both IPv4 and IPv6
     addresses.

so i assume inet_ntop is unavailable on some major os, correct?


> +    av_strlcpy(buf, res, len);
> +#endif
> +
> +    return buf;
> +}
> +
>  int ff_socket_nonblock(int socket, int enable)
>  {
>  #ifdef HAVE_WINSOCK2_H

> Index: libavformat/avformat.h
> ===================================================================
> --- libavformat/avformat.h.orig	2007-09-29 11:22:01.000000000 -0400
> +++ libavformat/avformat.h	2007-09-29 11:38:21.000000000 -0400
> @@ -21,8 +21,8 @@
>  #ifndef AVFORMAT_H
>  #define AVFORMAT_H
>  
> -#define LIBAVFORMAT_VERSION_INT ((51<<16)+(14<<8)+0)
> -#define LIBAVFORMAT_VERSION     51.14.0
> +#define LIBAVFORMAT_VERSION_INT ((51<<16)+(15<<8)+0)
> +#define LIBAVFORMAT_VERSION     51.15.0
>  #define LIBAVFORMAT_BUILD       LIBAVFORMAT_VERSION_INT

this is not needed as there is no change to the public API/ABI


[...]

> + * @param port the port number that will be connected to.
> + * @param passive 1 if the port will be used to bind() to, else 0.
> + * @param socket_stream 1 if the returned address will be used for a
> + *                      TCP/IP stream-based connection, 0 for others.
> + * @param parse_only 1 if we should only parse numeric IPv4/6 host
> + *                   addresses without resolving it using a name server.

i think it would be more readable to use int flags here that is

blah(0,1,1) is harder to read than blah(AV_RESOLVE_FOO|AV_RESOLVE_BAR)
as the reader has too lookup what the argument 1,2,3 is to make sense of the
first, while the name in the second makes it immedeatly clear


[...]

> +/**
> + * Allocate an AVInetAddr for use in accept().
> + *
> + * @param socket_stream 1 if the address is used for TCP/IP stream-based
> +                        connections, 0 for others.
> + * return the newly allocated AVInetAddr, or NULL on ENOMEM.

@return

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070930/b21f67db/attachment.pgp>