Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 27 Jul 2006 08:22:11 -0700
From:      Sam Leffler <sam@errno.com>
To:        Robert Watson <rwatson@FreeBSD.org>
Cc:        cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/dev/usb if_ural.c
Message-ID:  <44C8DA23.5000406@errno.com>
In-Reply-To: <20060727124542.V4612@fledge.watson.org>
References:  <200607260330.k6Q3UooP047077@repoman.freebsd.org> <20060727124542.V4612@fledge.watson.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Robert Watson wrote:
> 
> On Wed, 26 Jul 2006, Sam Leffler wrote:
> 
>> sam         2006-07-26 03:30:50 UTC
>>
>>  FreeBSD src repository
>>
>>  Modified files:
>>    sys/dev/usb          if_ural.c
>>  Log:
>>  support for 802.11 packet injection via bpf
>>
>>  Reviewed by:    arch@
>>  MFC after:      1 month
> 
> ---------------------------------------
> @@ -209,6 +211,9 @@
>          */
>         if (ic->ic_reset == NULL)
>                 ic->ic_reset = ieee80211_default_reset;
> +
> +       KASSERT(ifp->if_spare2 == NULL, ("oops, hosed"));
> +       ifp->if_spare2 = ic;                    /* XXX temp backpointer */
>  }
> ---------------------------------------
> 
> Please don't use spare pointers without properly allocating them (i.e.,
> renaming them).  I ran into this this morning when merging these changes
> into the rwatson_ifnet branch, where if_spare2 was renamed to
> if_startmbuf. However, if the above change is MFC'd as is, people may
> well find the problem through run-time corruption when the network stack
> tries to execute the ic contents thinking that it's a non-NULL
> if_startmbuf function pointer, which is a lot harder to debug.  The
> point of spare fields is that they be spare -- if they are no longer
> spare, they should not be left marked as spare, or they will get reused
> with unfortunate ABI consequences.  I don't mind if this spare is
> allocated (if_softc2, if_driver, or the like), as we have one more spare
> pointer I can use in 6.x, but the current use of the spare field is
> problematic and should not be MFC'd unless cleaned up.

This was intended as a stopgap until the vap changes were in place and
will never be mfc'd.  I understand about reuse; hence the assertion.
Since you renamed the field didn't code just stop compiling?  I assumed
these were going to be treated like the M_PROTOx flags in mbuf.h that
are kinda spare but used within layers for their own purpose.

	Sam



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