From owner-freebsd-ports@FreeBSD.ORG Wed Mar 11 15:04:33 2009 Return-Path: Delivered-To: freebsd-ports@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3873B1065672 for ; Wed, 11 Mar 2009 15:04:33 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 3CC8D8FC1A for ; Wed, 11 Mar 2009 15:04:31 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id RAA04007; Wed, 11 Mar 2009 17:04:30 +0200 (EET) (envelope-from avg@icyb.net.ua) Message-ID: <49B7D2FE.5040901@icyb.net.ua> Date: Wed, 11 Mar 2009 17:04:30 +0200 From: Andriy Gapon User-Agent: Thunderbird 2.0.0.19 (X11/20090110) MIME-Version: 1.0 To: Wesley Shields References: <49B6A827.50705@icyb.net.ua> <20090311145202.GA85211@atarininja.org> In-Reply-To: <20090311145202.GA85211@atarininja.org> X-Enigmail-Version: 0.95.7 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: freebsd-ports@FreeBSD.org Subject: Re: request for a new port review [memtest86+] X-BeenThere: freebsd-ports@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting software to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Mar 2009 15:04:33 -0000 on 11/03/2009 16:52 Wesley Shields said the following: > On Tue, Mar 10, 2009 at 07:49:27PM +0200, Andriy Gapon wrote: >> Guys, >> >> could you please review the below port for correctness, style and >> general approach taken by me. This is a port of memtest86+. Unlike >> existing sysutils/memtest86 this port is not a >> download/extraction/version-tracking aid, rather it is designed to >> build a stand-alone ELF image (bootable by boot2 or loader) and/or an >> ISO image from sources. The first option does not need any additional >> justification, I think. The second option can be useful if you want to >> add some local patches on top of vendor sources. >> >> I am very grateful for the idea for this port and many technical >> details of it to Stephan Eisvogel. I also thank Eygene Ryabinkin for >> teaching me some things about ld. >> >> Alternatives for /boot/opt are welcome :) >> >> P.S. Stephan, I plan to create a distinct port for your version very >> soon. I am also considering making it a port option for this proposed >> port, the option that would apply an extra patch. > > You seem to be using two tabs on every line. I think you should use > only one. Just a style nit. Good point, I used to have a variable with a long name but it's gone, so I'll condense the assignments. > I don't know if /boot/opt is the best place for it. It seems like > /boot/local fits in better with the concept of /usr/local. Of course, > it's just a name and I'm not attached strongly to any of them. Me neither. I opted for "opt" :-) because it's shorter and right now there is no auto-completion for boot2 or loader prompts. > What's wrong with making these changes to the existing memtest port, as > OPTIONS? Two reasons: (1) memtest seems to be tailored to a different purpose and it currently has a maintainer whom I haven't consulted about any changes yet; (2) [more important] original memtest86 has a peculiarity that it needs to be loaded in conventional memory (first 640k) and that doesn't work very well with our loader (yet); I have some ideas, but they need additional work, so memtest86+ was the lower-hanging fruit. -- Andriy Gapon