From owner-freebsd-geom@FreeBSD.ORG Sat Apr 14 16:33:56 2007 Return-Path: X-Original-To: freebsd-geom@freebsd.org Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id D8FD316A401; Sat, 14 Apr 2007 16:33:56 +0000 (UTC) (envelope-from rink@thunderstone.rink.nu) Received: from mx1.rink.nu (thunderstone.rink.nu [80.112.228.34]) by mx1.freebsd.org (Postfix) with ESMTP id 9930213C45D; Sat, 14 Apr 2007 16:33:56 +0000 (UTC) (envelope-from rink@thunderstone.rink.nu) Received: from localhost (localhost [127.0.0.1]) by mx1.rink.nu (Postfix) with ESMTP id 3C0C2D4C61; Sat, 14 Apr 2007 18:06:38 +0200 (CEST) X-Virus-Scanned: amavisd-new at rink.nu Received: from mx1.rink.nu ([127.0.0.1]) by localhost (thunderstone.rink.nu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ns0SdPW3davc; Sat, 14 Apr 2007 18:06:30 +0200 (CEST) Received: from thunderstone.rink.nu (localhost [127.0.0.1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.rink.nu (Postfix) with ESMTP id EC05CD4C60; Sat, 14 Apr 2007 18:06:29 +0200 (CEST) Received: (from rink@localhost) by thunderstone.rink.nu (8.13.8/8.13.8/Submit) id l3EG6TsY084820; Sat, 14 Apr 2007 18:06:29 +0200 (CEST) (envelope-from rink) Date: Sat, 14 Apr 2007 18:06:29 +0200 From: Rink Springer To: Ed Schouten Message-ID: <20070414160629.GA82214@rink.nu> References: <20070414115959.GN81821@hoeg.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <20070414115959.GN81821@hoeg.nl> User-Agent: Mutt/1.5.13 (2006-08-11) Cc: Rink Springer , Laurens Timmermans , FreeBSD GEOM Subject: Re: New GEOM module: geom_xboxhd 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: Sat, 14 Apr 2007 16:33:56 -0000 Hi Ed, On Sat, Apr 14, 2007 at 01:59:59PM +0200, Ed Schouten wrote: > I've placed the code in a Git repository, which can be viewed online: >=20 > http://g-rave.nl/gitweb?p=3Dgeom_xboxhd;a=3Dtree Some comments: (1) g_xboxhd_checkmagic(): You look at uint32_t* buf, which you have just g_free()'d; this seems wrong. I know this is debugging code, but you could just get rid of these lines all the same ;-) (2) I consider the for-loop in line 125 a bit kludgy - why not add all 'standard' partitions (of which you know they are always there) in this loop, and if the disk was larger, add a final paritition containing the rest? This makes the code prettier to read, in my opinion. (3) This may be personal preference, but I consider a check of 4 bytes on address 0x600 a bit weak - usually, the first 63 sectors or so after the boot sector are skipped during partitioning (most general boot managers reside there), so there may be no telling what's in there. However, there doesn't seem to be a better way to identify Xbox harddisks, which makes me wonder whether we should keep this in the XBOX kernel config file only, possibly commented out in LINT. Anyway, apart from this, it looks functional and something the xbox port could very well use. I'd like to get this in the tree, so people with more geom-fu are very welcome to take a look! Have you tried what sysinstall(8) does when it sees such a layout? It'd be a shame if it nuked the partition table... :-) Regards, --=20 Rink P.W. Springer - http://rink.nu "It is such a quiet thing, to fall. But yet a far more terrible thing, to admit it." - Darth Traya