Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 13 Oct 2010 13:59:42 -0400
From:      Jung-uk Kim <jkim@FreeBSD.org>
To:        Roman Divacky <rdivacky@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Rui Paulo <rpaulo@freebsd.org>
Subject:   Re: svn commit: r213793 - in head/sys/dev: ce cp
Message-ID:  <201010131359.44590.jkim@FreeBSD.org>
In-Reply-To: <20101013172757.GA21579@freebsd.org>
References:  <201010131717.o9DHHobD094458@svn.freebsd.org> <20101013172757.GA21579@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--Boundary-00=_QOftMZ1MEke0rjl
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

On Wednesday 13 October 2010 01:27 pm, Roman Divacky wrote:
> On Wed, Oct 13, 2010 at 05:17:50PM +0000, Rui Paulo wrote:
> > Author: rpaulo
> > Date: Wed Oct 13 17:17:50 2010
> > New Revision: 213793
> > URL: http://svn.freebsd.org/changeset/base/213793
> >
> > Log:
> >   Don't do a logical AND of the result of strcmp() with a
> > constant.
> >
> >   Found with:	clang
> >
> > Modified:
> >   head/sys/dev/ce/if_ce.c
> >   head/sys/dev/cp/if_cp.c
> >
> > Modified: head/sys/dev/ce/if_ce.c
> > =================================================================
> >============= --- head/sys/dev/ce/if_ce.c	Wed Oct 13 17:16:08
> > 2010	(r213792) +++ head/sys/dev/ce/if_ce.c	Wed Oct 13 17:17:50
> > 2010	(r213793) @@ -1313,7 +1313,7 @@ static int ce_ioctl (struct
> > cdev *dev, u IFP2SP(d->ifp)->pp_flags &= ~(PP_FR);
> >  			IFP2SP(d->ifp)->pp_flags |= PP_KEEPALIVE;
> >  			d->ifp->if_flags |= PP_CISCO;
> > -		} else if (! strcmp ("fr", (char*)data) && PP_FR) {
> > +		} else if (! strcmp ("fr", (char*)data)) {
>
> this is wrong I think... the PP_FR was used for compiling in/out
> support for something.. see the comment:
>
> /* If we don't have Cronyx's sppp version, we don't have fr support
> via sppp */ #ifndef PP_FR
> #define PP_FR 0
> #endif
>
> note that PP_FR is used in some other places as a flag. I guess
> that by compiling with something like make -DPP_FR=42 some magic
> would happen.
>
> anyway - this does not look like a bug but like an intent, please
> revert.

I think the attached patch should do.

Jung-uk Kim

--Boundary-00=_QOftMZ1MEke0rjl
Content-Type: text/plain;
  charset="iso-8859-1";
  name="cronyx.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="cronyx.diff"

Index: sys/dev/ce/if_ce.c
===================================================================
--- sys/dev/ce/if_ce.c	(revision 213782)
+++ sys/dev/ce/if_ce.c	(working copy)
@@ -1313,9 +1313,11 @@ static int ce_ioctl (struct cdev *dev, u_long cmd,
 			IFP2SP(d->ifp)->pp_flags &= ~(PP_FR);
 			IFP2SP(d->ifp)->pp_flags |= PP_KEEPALIVE;
 			d->ifp->if_flags |= PP_CISCO;
-		} else if (! strcmp ("fr", (char*)data) && PP_FR) {
+#if PP_FR > 0
+		} else if (! strcmp ("fr", (char*)data)) {
 			d->ifp->if_flags &= ~(PP_CISCO);
 			IFP2SP(d->ifp)->pp_flags |= PP_FR | PP_KEEPALIVE;
+#endif
 		} else if (! strcmp ("ppp", (char*)data)) {
 			IFP2SP(d->ifp)->pp_flags &= ~PP_FR;
 			IFP2SP(d->ifp)->pp_flags &= ~PP_KEEPALIVE;
Index: sys/dev/cp/if_cp.c
===================================================================
--- sys/dev/cp/if_cp.c	(revision 213782)
+++ sys/dev/cp/if_cp.c	(working copy)
@@ -1052,9 +1052,11 @@ static int cp_ioctl (struct cdev *dev, u_long cmd,
 			IFP2SP(d->ifp)->pp_flags &= ~(PP_FR);
 			IFP2SP(d->ifp)->pp_flags |= PP_KEEPALIVE;
 			d->ifp->if_flags |= PP_CISCO;
-		} else if (! strcmp ("fr", (char*)data) && PP_FR) {
+#if PP_FR > 0
+		} else if (! strcmp ("fr", (char*)data)) {
 			d->ifp->if_flags &= ~(PP_CISCO);
 			IFP2SP(d->ifp)->pp_flags |= PP_FR | PP_KEEPALIVE;
+#endif
 		} else if (! strcmp ("ppp", (char*)data)) {
 			IFP2SP(d->ifp)->pp_flags &= ~PP_FR;
 			IFP2SP(d->ifp)->pp_flags &= ~PP_KEEPALIVE;

--Boundary-00=_QOftMZ1MEke0rjl--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201010131359.44590.jkim>