Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 7 Feb 2001 15:28:49 +0600 (ALMT)
From:      Boris Popov <bp@butya.kz>
To:        Bosko Milekic <bmilekic@technokratis.com>
Cc:        freebsd-arch@FreeBSD.ORG, freebsd-net@FreeBSD.ORG
Subject:   Re: Sequential mbuf read/write extensions
Message-ID:  <Pine.BSF.4.21.0102071516110.7952-100000@lion.butya.kz>
In-Reply-To: <003001c090b8$0b067a50$1f90c918@jehovah>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 6 Feb 2001, Bosko Milekic wrote:

> > Since currently there isn't many consumers of this code I can
> > suggest to define an option LIBMBUF in the kernel configuration file
> and
> > add KLD libmbuf (with interface libmbuf), so kernel footprint will
> not be
> 
>     I am in favor of such an option on the condition that it is
> temporary. In other words, only until we decide "we have converted
> enough code to use this code so we should remove the option now." The
> reason is that otherwise, we will be faced with numerous "#ifdef
> LIBMBUF ... #else ... #endif" code. I assume this is what you meant,

	Not exactly so. 'option LIBMBUF' will just connect the source file
to kernel makefile. There is no need for any #ifdef's in the code.

> #define M_TRYWAIT M_WAIT is not right.
> (M_WAIT is no longer to be used in the mbuf code.)

	You omitted the surrounding "#ifndef M_TRYWAIT" which makes this
code portable to RELENG_4 (mind you, this code taken from smbfs). Of
course, this should be stripped before import.

> The succesfull return values are 0, I don't have a problem with this,
> specifically, but I would assume that this:
> if (!mb_init(mbp))  ... would be more "logical" (I use the term
> loosely) if it meant: "if initialization fails" (now it means "if
> initialization is succesful").

	I'm generally don't like such syntax if function or variable name
do not clearly specify which value it should have/return on success. 
Nearly all functions in this file return zero or error code, so the
correct syntax of the above will be:

	error = mb_init(mbp);
	if (!error)

or

	if (error)
		return error;

or

	if (mb_init(mbp) != 0)
		return ESOMETHINGEVIL;

> > significantly affected. The names of source and header files are
> > questionable too and I would appreciate good suggestions (currently
> they
> > are subr_mbuf.c and subr_mbuf.h).
> 
>     Hmmm. Maybe subr_mblib.c and libmb.h ? I don't want to turn this
> into a bikeshed ( :-) ), so I suggest that you decide. Personally, I
> would prefer that it be something other than "subr_mbuf.c" simply
> because it may be a little misleading in some cases.

	Good point.

> Boris, this is really a great interface and nice looking, clean code.

	I'm sure, this code can be significantly improved by mbuf gurus :)

--
Boris Popov
http://www.butya.kz/~bp/



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?Pine.BSF.4.21.0102071516110.7952-100000>