From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1MF9W7-0005YC-5I for mharc-grub-devel@gnu.org; Fri, 12 Jun 2009 12:22:11 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MF9W5-0005Xs-GM for grub-devel@gnu.org; Fri, 12 Jun 2009 12:22:09 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MF9W0-0005Wq-UX for grub-devel@gnu.org; Fri, 12 Jun 2009 12:22:08 -0400 Received: from [199.232.76.173] (port=56132 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MF9W0-0005Wg-Fl for grub-devel@gnu.org; Fri, 12 Jun 2009 12:22:04 -0400 Received: from c60.cesmail.net ([216.154.195.49]:48624) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.60) (envelope-from ) id 1MF9Vz-0004S9-GM for grub-devel@gnu.org; Fri, 12 Jun 2009 12:22:04 -0400 Received: from unknown (HELO smtprelay2.cesmail.net) ([192.168.1.112]) by c60.cesmail.net with ESMTP; 12 Jun 2009 12:21:56 -0400 Received: from [192.168.0.22] (static-72-92-88-10.phlapa.fios.verizon.net [72.92.88.10]) by smtprelay2.cesmail.net (Postfix) with ESMTPSA id 79A4234C6D for ; Fri, 12 Jun 2009 12:26:35 -0400 (EDT) From: Pavel Roskin To: The development of GRUB 2 In-Reply-To: <1244802503.3181.0.camel@fz.local> References: <1241620317.3746.8.camel@fz.local> <1243885166.3417.11.camel@fz.local> <1244496026.3833.37.camel@fz.local> <1244674821.8525.23.camel@fz.local> <1244762505.3552.64.camel@fz.local> <1244762744.3552.65.camel@fz.local> <1244802503.3181.0.camel@fz.local> Content-Type: text/plain Date: Fri, 12 Jun 2009 12:21:54 -0400 Message-Id: <1244823714.9051.42.camel@mj> Mime-Version: 1.0 X-Mailer: Evolution 2.26.2 (2.26.2-1.fc11) Content-Transfer-Encoding: 7bit X-detected-operating-system: by monty-python.gnu.org: Genre and OS details not recognized. Subject: Re: [PATCH] Re: grub-install --root-directory=/mnt /dev/sda1 fails X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Jun 2009 16:22:09 -0000 On Fri, 2009-06-12 at 12:28 +0200, Felix Zielcke wrote: > Ok here's a new one which compiles without warnings. I suggest that whenever a patch is published, it comes with a detailed description. Surely, it could be gathered by rereading the thread, but I think it's not sufficient for two reasons. The first reason is that we want the patches reviewed by more people. Not everybody has time to read the whole discussion. The second reason is that the description the shows how the author sees the changes, what the potential benefits are, what code is affected. The reviewers would be able to compare the goals to the implementation. Even if the description was already published, it should not be hard to publish it again, perhaps with some improvements. Regarding this patch, I don't think we need to add a function to hostdisk.c is it's only used in grub-setup.c. It would be better to have it in grub-setup.c unless it's a generic function or there is a good chance that it will be used elsewhere. grub_make_system_path_relative_to_its_root is a very long name. There is nothing wrong with a long name per se, but it may be in sign that the function is too specialized and should not be in hostdisk.c. Following is not good for several reasons: #warning "The function `grub_make_system_path_relative_to_its_root' might not work on your OS correctly." The warning will only be seen by those who compile GRUB, but not by the end users who installed a binary. The warning doesn't explain what exactly may not work correctly. Since we are talking about new functionality here, I'd rather omit an unsafe implementation if realpath() is not available. What is free_ptr? It looks like it's a pointer that the caller should free. I think functions should be self-contained and should not ask the caller to do the cleanup (unless they actually return something useful in the allocated memory). -- Regards, Pavel Roskin