[FFmpeg-devel] [PATCH] IPv6 support v.2

[Nicolas George]

L'octidi 18 brumaire, an CCXVI, Ronald S. Bultje a ?crit?:
> Only those two are supported anyway.

I think a goal to set for that code should be that supporting a new address
family would not require anything outside a few wrapper functions.

>				       Calls like getaddrinfo() don't support
> any others.

But they may. There have been implementations of getaddrinfo that could
return AF_UNIX addresses, for example. And there may be implementations
supporting new or old protocols.

>	      But I can sort of see your point, I could add a macro like
> AV_RESOLVE_FAMILY(addr) which sets the flag:
> 
> #define AV_RESOLVE_FAMILY(addr) \
>     addr->addr->sa_family == AF_INET ? AV_RESOLVE_IPV4ONLY :
>     (addr->addr->sa_family == AF_INET6 ? AV_RESOLVE_IPV6ONLY : 0)
> 
> A reverse macro for use inside inetaddr_resolve() could also be done,
> although I'd consider it overkill already. Would that be OK for you?

The more I think about it, the less I am happy with these AV_RESOLVE_*ONLY
flags. What is the reason for their presence? I see 3 possible:

a. the address will be used by a subsystem that only handles one address
   family (and can not easily fixed to be protocol-independent);

b. the address needs to be compatible with another, for example for a
   connect/bind pair;

c. the user explicitly requested so.

For a and b, using the AF_* value itself is fine: in a, the constant we need
is necessarily available, and in b, the value is in the address structure of
the address we need to be compatible with.

(And anyway, for b, I as pointed earlier, restricting the address family is
not correct.)

For c, we need constants that do not depend on system headers, but I believe
that a string constant would be preferable.

> Your macro is equally fine with me, and does indeed look safer. I think the
> header mess is why I did it this way. We can make two macros, one for if
> it's there (inet6/in6.h) and one for if it isn't, it's not that bad.

To avoid the protocol mess, the best is probably to avoid macros and use
accessing functions.

By the way, the correct header for sockaddr_in6 is netinet/in.h; any system
which works otherwise is broken with regard with the standard.

> It's for ffserver, it does ACL checks using this thing.

Could these thecks be done on the string representation rather than the
binary representation?

>							  Other than that, it
> shouldn't be used, and it isn't used, except in inet_ntop() inside
> inetaddr_str() (and yes that's how you're supposed to do it, apparently,
> it's a bit ugly... Ohwell).

I had the impression that getnameinfo (with NI_NUMERICHOST and
NI_NUMERICSERV) was the recommended API. Did you try so?

Regards,

-- 
  Nicolas George