From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Ng1y2-0007K1-JM for qemu-devel@nongnu.org; Fri, 12 Feb 2010 15:18:22 -0500 Received: from [199.232.76.173] (port=54435 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ng1y2-0007Jr-8y for qemu-devel@nongnu.org; Fri, 12 Feb 2010 15:18:22 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Ng1y1-0008R0-O8 for qemu-devel@nongnu.org; Fri, 12 Feb 2010 15:18:22 -0500 Received: from are.twiddle.net ([75.149.56.221]:47407) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Ng1wb-0008LW-RA for qemu-devel@nongnu.org; Fri, 12 Feb 2010 15:18:21 -0500 Message-ID: <4B75B725.1040305@twiddle.net> Date: Fri, 12 Feb 2010 12:16:37 -0800 From: Richard Henderson MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 3/6] Fix last page errors in page_set_flags and page_check_range. References: In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel@nongnu.org On 02/12/2010 11:47 AM, Blue Swirl wrote: > Please make separate patches for unrelated changes. Now the essence of > the patch is very hard to see. Also pure formatting changes are not > very useful. Is this just about page_get_flags, or was there some other pure formatting change to which you object? For instance: >> - if (start + len < start) >> - /* we've wrapped around */ >> + if (start + len - 1 < start) { >> + /* We've wrapped around. */ >> return -1; >> + } Only the first line is required to fix an off-by-one error. But if I don't add the braces, generally the patch will be bounced for that. >> - /* We may be called for host regions that are outside guest >> - address space. */ > > Why remove the comment, is it no longer true? Yes, after the entire patch series is applied. Indeed, I believe that many of the addresses rejected here were *not* in fact outside the guest address space, merely that page_find_alloc did not accurately know what the guest address space was. I think it was a mistake to ever consider usage of this function with out-of-band addresses as valid -- it adds a great deal of confusion. I could add an assertion here to make sure, if you like. If this were C++, it might have been interesting to try to use the type system to keep the host and guest address spaces forcibly separate. But since this is plain C, I think wrapping everything in structures and access macros would just be too ugly. r~