Date: Thu, 02 Jun 2005 20:41:39 +0200 From: Andre Oppermann <andre@freebsd.org> To: "Li, Qing" <qing.li@bluecoat.com> Cc: freebsd-net@freebsd.org Subject: Re: issue with route Message-ID: <429F52E3.53FB6C9@freebsd.org> References: <48D44BB27BDE3840BDF18E59CB169A5C010AF884@bcs-mail3.internal.cacheflow.com>
next in thread | previous in thread | raw e-mail | index | archive | help
"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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?429F52E3.53FB6C9>