Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 7 Mar 2003 16:51:39 +0200
From:      Giorgos Keramidas <keramida@ceid.upatras.gr>
To:        Hiten Pandya <hiten@unixdaemons.com>
Cc:        arch@freebsd.org
Subject:   Re: Using m_getcl() in network and nfs code paths
Message-ID:  <20030307145139.GD2094@gothmog.gr>
In-Reply-To: <20030307092256.GA69971@unixdaemons.com>
References:  <20030307004958.GA98917@unixdaemons.com> <20030306212638.A32850@xorpc.icir.org> <20030307080659.GA60937@unixdaemons.com> <20030307002525.A50491@xorpc.icir.org> <20030307092256.GA69971@unixdaemons.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On 2003-03-07 04:22, Hiten Pandya <hiten@unixdaemons.com> wrote:
> Luigi Rizzo (Fri, Mar 07, 2003 at 12:25:25AM -0800) wrote:
> > the logic of this code
> >
> >  	m = (n > X) ? m_getcl(...) : m_get(...)
> >
> > The following: i have n bytes of data, give me a place large
> > enough to store them. This can be either a single mbuf or an
> > mbuf+cluster, depending on the size. The threshold (X) is whatever
> > fits into the mbuf (which varies depending on whether or not this is
> > a pkthdr mbuf, but again this is easy to tell inside m_getcl because
> > you are passing the M_PKTHDR flag).
> >
> > Now, MINCLSIZE/MHLEN are basically the same thing, and MLEN covers
> > the case for !M_PKTHDR. But my point is that the programmer should
> > not bother to know which one to use and instead just let the function
> > do the right thing.  Fewer chances for bugs, and smaller code.
>
> 	Right.  I guess it makes better sense.  I will try and come up
> 	with these changes over the weekend, or maybe even today if I
> 	get the time.
>
> 	Cheers Luigi.

Can you also convert all those (condition) ? true : false; things to
(cleaner, in my opinion) if {...} else {...} pairs?  It really hurts
my eyes trying to read those ?: things, and writing these as:

	if (n > X)
		m_getcl(...);
	else
		m_get(...);

has the added benefit that whenever one needs to fix just code of the
`else' part, the diff output of the changes will be much much cleaner
in the future.  This is, of course, more related to style than the
real architectural characteristics of the code.  But I just thought
I'd mention it, since I was too striken fairly fast by the same
impression that Luigi mentioned in his first post.

The argument of rewriting code to make things easier for possible
future changes is, admittedly, a bit weak.  But you can certainly
forget about all this if it's too much trouble :)

- Giorgos


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




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