Date: Sat, 6 Jan 2001 19:43:05 -0500 From: "Bosko Milekic" <bmilekic@technokratis.com> To: "Alfred Perlstein" <bright@wintelcom.net> Cc: <cvs-committers@FreeBSD.org>, <cvs-all@FreeBSD.org> Subject: Re: cvs commit: src/sys/dev/musycc musycc.c Message-ID: <003c01c07842$cc648bd0$25cbca18@jehovah> References: <200101062044.f06Kiex42615@freefall.freebsd.org> <20010106132943.E15744@fw.wintelcom.net> <005001c07839$40f06820$25cbca18@jehovah> <20010106161427.G15744@fw.wintelcom.net>
next in thread | previous in thread | raw e-mail | index | archive | help
See below.
Alfred wrote:
> * Bosko Milekic <bmilekic@technokratis.com> [010106 15:33] wrote:
> >
> > I find this incredibly dumb. You're just wasting a check that need not be
> > made. I know when I have to free m and when not to, it's dumb to have to
> > check it again in errfree. It's Poul's call, however.
>
> You have an exceptional condition, it reduces the amount of code
> someone reading must digest and it should reduce the amount of code
> the processor needs to put in the I cache.
The problem with it is that not all error conditions should lead to this
exit path. For example, if you fail before the allocation of the mbuf loop,
this is certainly not how you want to error. Also, if you fail before you
MALLOC those buffers, you certainly don't want to try FREE()ing them. I
suppose you could turn every single error condition in that function to jump
to some label at the bottom of the function. If anything, I find _that_ to be
more obfuscating the code than status quo.
> There is also a lot of code that already does this:
>
> sosend(), soreceive(), soclose().
Huh? What does sosend, soreceive, soclose have anything to do with this?
Or am I misunderstanding?
> What you're doing is giving a place to jump to further along in
> case any other error conditions come up.
What you are doing by moving the m_free down to the bottom is actually
growing the number of instructions that are cached. In the first case, where
you m_free at the top before the jump, you simply m_free in that case. In
your suggestion, you have the added instructions that account for checking
whether m is NULL, something you already know before jumping.
>
[...]
> --
> -Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
> "I have the heart of a child; I keep it in a jar on my desk."
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?003c01c07842$cc648bd0$25cbca18>