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

[Ronald S. Bultje]

Hi,

On Nov 8, 2007 5:08 PM, Nicolas George <nicolas.george at normalesup.org>
wrote:

> 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.
>

That is indeed my goal also. However, I see a macro as API, and the current
ones are sufficient for the current use-cases, and can be extended for
later.


> >                                      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.
>

Imo, if they're not part of the standard and implemented in all widely
available ones, we shouldn't rely on them at all.


> >             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.


Only (b). (c) could be interesting, but isn't my goal an isn't implemented
(yet). (a) won't happen.


> 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.


It's used only once (literally). Adding one extra argument for single-time
use is stupid, hence the flag insitead of extra function argument.


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


I don't quite see why. This is exactly what is documented to do (?).


> > 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.


That's an opinion, and I do not share it. :-). If you want to do it, then
please submit patches. I think I mentioned this before, I've had those for
over a year now, get them in, don't continuously start over from the
beginning, it doesn't get me anywhere.


> > 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?
>

"Patches welcome". I guess there's ways to do it. Since this works, I don't
care so much.


> >                                                         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?
>

Both work equally fine.

Ronald