On Wed, 26 Jul 2006, Christopher Faylor wrote: > Thanks for the patch,
Thanks for the review. > but I'm not convinced that this patch duplicates the functionality that > you eliminated from check_iovec. It doesn't exactly, but the part it doesn't didn't seem correct. See below. > And, the dummytest is actually there for a reason. Ok, then what *is* the reason for checking only the last byte of each iovec buffer for read or write-ability? Doesn't it need to check a byte on every system page to be complete (because buffers could start in or span across invalid/protected virtual addresses)? Should an error be flagged if the readv wouldn't have actually accessed the invalid addresses (I'm not sure here)? After my patch, if the iovec buffer addresses are invalid, they will either be flagged as such by the underlying Windows system call, or they will trigger the fault handler installed above by all check_iovec callers if the cygwin DLL code tries to access them, no? > It is not "more straighforward" to move a check out of a function and > duplicate it in callers of the function. Agreed in general, and straight forward may have been a poor choice of words. However, the other two callers [send|recv]msg already needed this type of fault handling for other reasons. So, to avoid doubling up there, and given the reasoning above, I thought that being consistent, covering a few missed (but admittedly rare cases), and a minuscule performance/elegance improvement to the general case justified the change. Maybe not. -- Brian Ford Lead Realtime Software Engineer VITAL - Visual Simulation Systems FlightSafety International the best safety device in any aircraft is a well-trained crew...
