From owner-freebsd-arch  Wed Feb  7  1:29:26 2001
Delivered-To: freebsd-arch@freebsd.org
Received: from relay.butya.kz (butya-gw.butya.kz [212.154.129.94])
	by hub.freebsd.org (Postfix) with ESMTP
	id 7863E37B684; Wed,  7 Feb 2001 01:28:59 -0800 (PST)
Received: by relay.butya.kz (Postfix, from userid 1000)
	id E5B812868D; Wed,  7 Feb 2001 15:28:49 +0600 (ALMT)
Received: from localhost (localhost [127.0.0.1])
	by relay.butya.kz (Postfix) with ESMTP
	id D66D82868A; Wed,  7 Feb 2001 15:28:49 +0600 (ALMT)
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
In-Reply-To: <003001c090b8$0b067a50$1f90c918@jehovah>
Message-ID: <Pine.BSF.4.21.0102071516110.7952-100000@lion.butya.kz>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: owner-freebsd-arch@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

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-arch" in the body of the message