This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: check_iovec cleanup


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...


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]