From owner-freebsd-hackers@FreeBSD.ORG Mon Nov 8 15:05:09 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 33481106566C for ; Mon, 8 Nov 2010 15:05:09 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id F13328FC0C for ; Mon, 8 Nov 2010 15:05:08 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 9E58246B06; Mon, 8 Nov 2010 10:05:08 -0500 (EST) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 8054B8A01D; Mon, 8 Nov 2010 10:05:07 -0500 (EST) From: John Baldwin To: Garrett Cooper Date: Mon, 8 Nov 2010 09:58:39 -0500 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201011080958.39156.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Mon, 08 Nov 2010 10:05:07 -0500 (EST) X-Virus-Scanned: clamav-milter 0.96.3 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-1.9 required=4.2 tests=BAYES_00 autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on bigwig.baldwin.cx Cc: freebsd-hackers@freebsd.org Subject: Re: [PATCH] mptutil(8) - capture errors and percolate up to caller X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Nov 2010 15:05:09 -0000 On Saturday, November 06, 2010 4:13:23 am Garrett Cooper wrote: > Similar to r214396, this patch deals with properly capturing error > and passing it up to the caller in mptutil just in case the errno > value gets stomped on by warn*(3); this patch deals with an improper > use of warn(3), and also some malloc(3) errors, as well as shrink down > some static buffers to fit the data being output. > If someone could review and help me commit this patch it would be > much appreciated; all I could do is run negative tests on my local box > and minor positive tests on my vmware fusion instance because it > doesn't fully emulate a fully working mpt(4) device (the vmware > instance consistently crashed with a warning about the mpt > controller's unimplemented features after I poked at it enough). > I'll submit another patch to fix up style(9) in this app if requested. > Thanks! The explicit 'return (ENOMEM)' calls are fine as-is. I do not think they need changing. Having static char arrays of '15' rather than '16' is probably pointless. The stack is already at least 4-byte aligned on all the architectures we support, so a 15-byte char array will actually be 16 bytes. It was chose to be a good enough value, not an exact fit. An exact fit is not important here. Moving the 'buf' in mpt_raid_level() is a style bug. It should stay where it is. Same with 'buf' in mpt_volstate() and mpt_pdstate(). IOC_STATUS_SUCCESS() returns a boolean, it is appropriate to test it with ! rather than == 0. It is also easier for a person to read the code that way. -- John Baldwin