issue with route

[Andre Oppermann]

"Li, Qing" wrote:
> 
> >
> > Please post unified diffs, they are far easier to read for humans.
> >
> 
>     Sorry, here is the patch again.
> 
>         -- Qing
> 
> Index: route.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/net/route.c,v
> retrieving revision 1.108
> diff -u -r1.108 route.c
> --- route.c     7 Jan 2005 01:45:35 -0000       1.108
> +++ route.c     2 Jun 2005 17:49:28 -0000
> @@ -743,8 +743,12 @@
>                 goto makeroute;
> 
>         case RTM_ADD:
> -               if ((flags & RTF_GATEWAY) && !gateway)
> -                       panic("rtrequest: GATEWAY but no gateway");
> +                if (flags & RTF_GATEWAY) {
> +                    if (!gateway)
> +                        panic("rtrequest: GATEWAY but no gateway");
> +                if (dst && (dst->sa_family != gateway->sa_family))
> +                    senderr(EINVAL);
> +                }
> 
>                 if (info->rti_ifa == NULL && (error = rt_getifa(info)))
>                         senderr(error);

There are some style issues with you proposal.  For every "if" you need
to indent by one tab, like this:

        case RTM_ADD:
                if (flags & RTF_GATEWAY) {
                	if (!gateway)
                        	panic("rtrequest: GATEWAY but no gateway");
                	if (dst && (dst->sa_family != gateway->sa_family))
                    		senderr(EINVAL);
                }

                if (info->rti_ifa == NULL && (error = rt_getifa(info)))
                        senderr(error);

There are tab/whitespace issues as well, but I think that is due to you
using Outbreak as mailer.

Here is my alternative version with slightly expanded scope of the
gateway address family check.  I've converted the panic to an EINVAL
as well.  We don't want to panic the box if someone screws up a
routing socket message.

        case RTM_ADD:
                if ((flags & RTF_GATEWAY) && !gateway)
                        senderr(EINVAL);
                if (dst && gateway && (dst->sa_family != gateway->sa_family))
                        senderr(EINVAL);

                if (info->rti_ifa == NULL && (error = rt_getifa(info)))
                        senderr(error);
                ifa = info->rti_ifa;

Please check if my variant does the right thing in all cases.  If you
can confirm, then you can go ahead and write up a descriptive commit
message.  You can find some examples in the cvs log of sys/net/route.c:

 http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/net/route.c

Then please send me your final patch including proposed commit message
for final review again.  After that, when no more issues arise, you can
go ahead and commit the change.

Once you've been a few times through this precedure you get more and
more familiar with the FreeBSD-way of making code changes.  And after
a while, when you are fluent in FreeBSDism, you get on your own and out
of mentorship.

Oh, BTW.  Don't be afraid when you get brucified.  Bruce' style
comments are a very valuable learning resource.  Everyone of us got
brucified more than once.  ;-)  (I'm talking about Bruce Evans, bde@)

-- 
Andre