From owner-freebsd-net  Tue Oct 10 18:15:13 2000
Delivered-To: freebsd-net@freebsd.org
Received: from whistle.com (s205m131.whistle.com [207.76.205.131])
	by hub.freebsd.org (Postfix) with ESMTP id 5801737B66C
	for <freebsd-net@freebsd.org>; Tue, 10 Oct 2000 18:15:10 -0700 (PDT)
Received: (from smap@localhost)
	by whistle.com (8.10.0/8.10.0) id e9B1EZa15880;
	Tue, 10 Oct 2000 18:14:35 -0700 (PDT)
Received: from bubba.whistle.com( 207.76.205.7) by whistle.com via smap (V2.0)
	id xma015873; Tue, 10 Oct 2000 18:14:05 -0700
Received: (from archie@localhost)
	by bubba.whistle.com (8.11.0/8.11.0) id e9B1E0J40614;
	Tue, 10 Oct 2000 18:14:00 -0700 (PDT)
	(envelope-from archie)
From: Archie Cobbs <archie@whistle.com>
Message-Id: <200010110114.e9B1E0J40614@bubba.whistle.com>
Subject: Re: ip_input.c patch
In-Reply-To: <Pine.BSF.4.21.0010101850530.8597-100000@jehovah.technokratis.com>
 "from Bosko Milekic at Oct 10, 2000 07:11:05 pm"
To: Bosko Milekic <bmilekic@dsuper.net>
Date: Tue, 10 Oct 2000 18:14:00 -0700 (PDT)
Cc: freebsd-net@freebsd.org
X-Mailer: ELM [version 2.4ME+ PL82 (25)]
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII
Sender: owner-freebsd-net@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.org

Bosko Milekic writes:
> >  	/*
> > -	 * Convert fields to host representation.
> > +	 * Convert fields to host representation. But first make
> > +	 * sure we don't write into a multiply-referenced mbuf.
> >  	 */
> > +	if ((m->m_flags & M_EXT) != 0 && MEXT_IS_REF(m)
> > +	    && (m = m_pullup(m, sizeof(*ip))) == NULL) {
> 
> 	Assuming that you only want to attempt to pullup into a "multiply"
>   referenced mbuf, this check is OK.

Right.. we only need to pullup if (a) we're going to write into
the mbuf and (b) it's multiply referenced.

> > -		if (m->m_flags & M_EXT) {		/* XXX */
> > +		if ((m->m_flags & M_EXT) != 0 && MEXT_IS_REF(m)) {
> >  			if ((m = m_pullup(m, hlen)) == 0) {
> >  				ipstat.ips_toosmall++;
> 
> 	How about collapsing that m_pullup into the same if() statement, to
>   remain consistent with the above? The reason I'm suggesting you be picky
>   about this is that those relatively repetetive extensive checks on the
>   "readability" of the mbuf will likely soon be replaced ... as soon as I
>   merge a few diffs ( :-) ) and it will be simpler to search and replace
>   this way.

I think I'll wait for your diffs.

Uh oh.. I just thought of a plan.. :-)


  1. Add a new macro to make an mbuf writable:

     #define M_MKWRITABLE(m, len)
	do {
		if ((m)->m_len < (len)
		    || (((m)->m_flags & M_EXT) != 0 && MEXT_IS_REF(m))) {
			(m) = m_pullup((m), (len));
		}
	} while (0)

  2. Temporarily change the definition of mtod() as follows:

	BEFORE
	------

	    #define	mtod(m, t)	((t)((m)->m_data))

	AFTER
	-----

	    #define	mtod(m, t)	((const t)((m)->m_data))

  3. Compile LINT and find and fix every place that generates
     an error from the const-cast in mtod():

     (A) If the code doesn't need to modify the mbuf (probably
	 99% of the time), then change it like so:

	BEFORE
	------
	    struct ip *ip;
	    ip = mtod(m, struct ip *);

	AFTER
	------
	    const struct ip *ip;
	    ip = mtod(m, struct ip *);

     (B) If the code does modify the mbuf, insert M_MKWRITABLE()
	 at the appropriate point.

  4. Put mtod() back the way it was

-Archie

___________________________________________________________________________
Archie Cobbs   *   Whistle Communications, Inc.  *   http://www.whistle.com


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message