Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 10 Oct 2000 18:14:00 -0700 (PDT)
From:      Archie Cobbs <archie@whistle.com>
To:        Bosko Milekic <bmilekic@dsuper.net>
Cc:        freebsd-net@freebsd.org
Subject:   Re: ip_input.c patch
Message-ID:  <200010110114.e9B1E0J40614@bubba.whistle.com>
In-Reply-To: <Pine.BSF.4.21.0010101850530.8597-100000@jehovah.technokratis.com> "from Bosko Milekic at Oct 10, 2000 07:11:05 pm"

next in thread | previous in thread | raw e-mail | index | archive | help
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




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