Date: Sun, 16 Mar 2008 10:05:55 +0100 From: Ulf Lilleengen <lulf@stud.ntnu.no> To: "Rick C. Petty" <rick-freebsd@kiwi-computer.com> Cc: freebsd-geom@freebsd.org Subject: Re: [patch] geom_vinum platform fixes Message-ID: <20080316090554.GA1230@carrot.studby.ntnu.no> In-Reply-To: <20080313182257.GB14969@keira.kiwi-computer.com> References: <20080310052711.GA49676@keira.kiwi-computer.com> <20080313153551.82wlu8iio4088c44@webmail.ntnu.no> <20080313182257.GB14969@keira.kiwi-computer.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080316090554.GA1230>