From owner-cvs-src Fri Feb 21 16: 0:33 2003 Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id F00C037B401; Fri, 21 Feb 2003 16:00:30 -0800 (PST) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6E8AB43F75; Fri, 21 Feb 2003 16:00:30 -0800 (PST) (envelope-from mux@freebsd.org) Received: by elvis.mu.org (Postfix, from userid 1920) id 4A6B2AE272; Fri, 21 Feb 2003 16:00:30 -0800 (PST) Date: Sat, 22 Feb 2003 01:00:30 +0100 From: Maxime Henrion To: Nate Lawson 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: <20030222000030.GT60813@elvis.mu.org> References: <20030221222315.GR60813@elvis.mu.org> <20030221235701.GS60813@elvis.mu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20030221235701.GS60813@elvis.mu.org> User-Agent: Mutt/1.4i Sender: owner-cvs-src@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Maxime Henrion wrote: > Nate Lawson wrote: > > 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). > > Then let's follow the existing style in those drivers. > > > > 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. > > Fair enough. It would be nice to add a comment explaining this then so > that people don't wonder about it. Also, telling it to lint with a > /* UNREACH */ or something might make sense. s/UNREACH/NOTREACHED/ Maxime To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-src" in the body of the message