Date: Fri, 21 Feb 2003 15:51:46 -0800 (PST) From: Nate Lawson <nate@root.org> To: Maxime Henrion <mux@freebsd.org> Cc: src-committers@FreeBSD.org, cvs-src@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/pci if_xl.c Message-ID: <Pine.BSF.4.21.0302211523570.54211-100000@root.org> In-Reply-To: <20030221222315.GR60813@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 21 Feb 2003, Maxime Henrion wrote:
> Maxime Henrion wrote:
> These patches look mostly fine (I only looked at the busdma drivers). I
> saw two minor nits though. You're using "if (error != 0)", I personally
> prefer "if (error)" (I don't know if style(9) says something for this),
> but what's more important is that it makes style inconsistent in those
> drivers where the latter is used everywhere else.
I did it because of this example and the warning against "!error".
error = function(a1, a2);
if (error != 0)
exit(error);
But I don't care either way -- I try to stick to the original style when
possible (i.e. the xl changes do not remove the goto tree even though I
don't like that style).
> Also, there's an unreachable bus_teardown_intr() call in the xl(4) diff.
> It should be just removed since with your patches, the interrupt is the
> last thing allocated in the xl_init() function and thus we never need to
> deallocate it there.
I actually did that on purpose to handle future reordering if it
happens. I think it's an advantage to have the resource cleanup be
(mostly) independent of the order it was allocated in. In fact, with
a little more work, the attach error case and the detach case could be
unified.
-Nate
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?Pine.BSF.4.21.0302211523570.54211-100000>