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~
