From owner-freebsd-geom@FreeBSD.ORG Sun Mar 16 09:06:05 2008 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EE710106564A for ; Sun, 16 Mar 2008 09:06:05 +0000 (UTC) (envelope-from lulf@stud.ntnu.no) Received: from bene1.itea.ntnu.no (bene1.itea.ntnu.no [IPv6:2001:700:300:3::56]) by mx1.freebsd.org (Postfix) with ESMTP id 531588FC17 for ; Sun, 16 Mar 2008 09:06:05 +0000 (UTC) (envelope-from lulf@stud.ntnu.no) Received: from localhost (localhost [127.0.0.1]) by bene1.itea.ntnu.no (Postfix) with ESMTP id 0F3CC16C764; Sun, 16 Mar 2008 10:06:03 +0100 (CET) Received: from carrot.studby.ntnu.no (unknown [IPv6:2001:700:300:3::185]) by bene1.itea.ntnu.no (Postfix) with ESMTP id B6EBA16C57C; Sun, 16 Mar 2008 10:06:01 +0100 (CET) Date: Sun, 16 Mar 2008 10:05:55 +0100 From: Ulf Lilleengen To: "Rick C. Petty" Message-ID: <20080316090554.GA1230@carrot.studby.ntnu.no> Mail-Followup-To: "Rick C. Petty" , freebsd-geom@freebsd.org References: <20080310052711.GA49676@keira.kiwi-computer.com> <20080313153551.82wlu8iio4088c44@webmail.ntnu.no> <20080313182257.GB14969@keira.kiwi-computer.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20080313182257.GB14969@keira.kiwi-computer.com> User-Agent: Mutt/1.5.17 (2007-11-01) X-Virus-Scanned: Debian amavisd-new at bene1.itea.ntnu.no Cc: freebsd-geom@freebsd.org Subject: Re: [patch] geom_vinum platform fixes X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 16 Mar 2008 09:06:06 -0000 On tor, mar 13, 2008 at 12:22:57pm -0600, Rick C. Petty wrote: > On Thu, Mar 13, 2008 at 03:35:51PM +0100, lulf@stud.ntnu.no wrote: *SNIP* > > That being said and since there have been no objections to my suggestions > (and Ulf agrees with my changes), is a committer willing to review my > patch? le@ ? Mr. Lehey? Hello, I've reviewed the patch and done some modifications to it. I'll need some testing first though (I don't have a testbed right now since I'm travelling). Here are some notes: - Removed GV_VERSION. It is not used anywhere. - Avoid moving header includes around. - Declare functions static when used inside one file. - Style fixes. - In gv_legacy_header_type, you can't be guaranteed that the hdr fields is not always null (although I agree in practise its highly unlikely), but it's hard to check this any other way, so I think it's alright :) - Use macro values instead of hard-coded values when testing legacy type. - Pass data variable as argument rather than returning a header. - Avoid memory allocation where possible. All this have been "fixed" in the new patch. Please test if it still works as specified. In addition, I noticed a memory leak in gvinum when rediscovering a drive too, but I'll commit this separately, and the fix is included in this patch for testing. It should work, but I've not tested it yet, but I might be able to during the day. The revised patch can be found here: http://people.freebsd.org/~lulf/patches/gvinum/gvinum_platformfix.diff Please let me know how it fares, and thanks for your help btw :) -- Ulf Lilleengen