I can't speak for everyone, but splitting the clauses of an if statement
with ifdefs makes my eyes bleed.
I would personally prefer to see these replaced with a switch statment,
as used elsewhere in PF; however this would probably need a goto that
points to the end of the main switch() statment to avoid insanely
complex costructs. Wait, that already exists...
switch (pp->af) {
#ifdef INET
case AF_INET:
break;
#endif / * INET */
#idfed INET6
case AF_INET6:
break;
#endif /* INET6 */
default:
error = EAFNOSUPPORT;
goto fail;
}
On Sat, Oct 03, 2009 at 09:04:01PM +0400, Vadim Zhukov wrote:
> Hello all, especially network hackers.
>
> There are a few places in sys/net/pf_ioctl.c that look strange for me.
> They are like so:
>
> #ifndef INET
> if (pp->af == AF_INET) {
> error = EAFNOSUPPORT;
> break;
> }
> #endif /* INET */
> #ifndef INET6
> if (pp->af == AF_INET6) {
> error = EAFNOSUPPORT;
> break;
> }
> #endif /* INET6 */
>
> The problem is that newrule->af is not checked for anything except
> AF_INET and AF_INET6. I.e. any address family will be accepted by ioctl.
> It doesn't matter very much, because other routines will silently ignore
> unknown address families. But why report EAFNOTSUPPORT at all then?
>
> The sample patch is below (hope it will apply cleanly; I just extracted
> it from the another work doing now).