From owner-cvs-all Fri Feb 21 15:51:52 2003 Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0DEC337B401 for ; Fri, 21 Feb 2003 15:51:50 -0800 (PST) Received: from rootlabs.com (root.org [67.118.192.226]) by mx1.FreeBSD.org (Postfix) with SMTP id 5E97043FD7 for ; Fri, 21 Feb 2003 15:51:48 -0800 (PST) (envelope-from nate@rootlabs.com) Received: (qmail 54310 invoked by uid 1000); 21 Feb 2003 23:51:46 -0000 Date: Fri, 21 Feb 2003 15:51:46 -0800 (PST) From: Nate Lawson To: Maxime Henrion Cc: src-committers@FreeBSD.org, cvs-src@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/pci if_xl.c In-Reply-To: <20030221222315.GR60813@elvis.mu.org> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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