Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 4 Jan 2006 06:57:20 +1300
From:      Andrew Thompson <thompsa@freebsd.org>
To:        Pawel Jakub Dawidek <pjd@freebsd.org>
Cc:        cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org
Subject:   Re: cvs commit: src/sys/net if_bridge.c
Message-ID:  <20060103175720.GD41998@heff.fud.org.nz>
In-Reply-To: <20060103112156.GB8302@garage.freebsd.pl>
References:  <200601022302.k02N2hBV014825@repoman.freebsd.org> <20060103112156.GB8302@garage.freebsd.pl>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Jan 03, 2006 at 12:21:56PM +0100, Pawel Jakub Dawidek wrote:
> On Mon, Jan 02, 2006 at 11:02:43PM +0000, Andrew Thompson wrote:
> +> thompsa     2006-01-02 23:02:43 UTC
> +> 
> +>   FreeBSD src repository
> +> 
> +>   Modified files:
> +>     sys/net              if_bridge.c 
> +>   Log:
> +>   Fix a brain-o in the last commit, the conditional was always false.
> [...]
> +> -	if (flags & IFBAF_DYNAMIC)
> +> +	if ((flags & IFBAF_TYPEMASK) == IFBAF_DYNAMIC)
> 
> On first look, I thought it does exactly the same thing, but I checked
> the code and now I know it doesn't - IFBAF_DYNAMIC is 0x00.

Yes, I was surprised and a little peeved too. This has been inherited
from Open/NetBSD.

> Another example that giving 0 for a define which should represent a flag
> is a bad idea. The same problem we had in the past with M_NOWAIT.
> 
> You should consider changing it to some real value to avoid mistakes
> like this in the future or removing IFBAF_DYNAMIC entirely and changing
> such condition to 'if (!(flags & IFBAF_STATIC))'.

It may be best to leave IFBAF_STATIC as 0x01 and set IFBAF_DYNAMIC to
0x02, this would make the MFC less invasive. I'll get it sorted out.


thanks,
Andrew




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