Tim Ellison wrote:
On 29/Sep/2009 15:40, Oliver Deakin wrote:
Tim Ellison wrote:
Making size == 0 a special case that always succeeds (even if it is
returning a bogus address) bypasses all the other checks that the mmap
function call would have made.
For example, what if the file descriptor was invalid? or points to an
unmappable file? Then I would expect an IOException.
I think (from the JIRA) that we were getting EINVAL back from the mmap
call with size 0, and that's why it was fixed before making the mmap
call. It would be good to know what the RI does in the case where size
is 0 and we're also mapping an invalid file - do we get an exception
thrown? Perhaps Catherine can clarify?
Ok, I missed that. Then perhaps the special case should be on
PlatformAddressFactory#allocMap() so that we don't invoke the overhead
of managing the 'bogus' 0 as if it were a real address that needs
freeing etc.
Ok, that sounds reasonable - Ill take a look.
There is nothing in the man pages to say that size = 0 is invalid.
No, but in the JIRA it indicates that we receive an EINVAL return code
when passing in size 0, so perhaps it is just not in the man page. The
man page I am looking at also didn't explicitly say negative values are
illegal, but I think we can assume that's true.
The man pages I'm looking at declare size as a (size_t) which is usually
unsigned ;-)
haha, ok that's true ;)
Plus, there is now no path that will return -1 as the address, so you
can remove the check (all the exceptions are raised in the native code).
That's true, I hadn't spotted that. I also noticed a bug in the native
code change where we allocate 100 chars for the error string but the
EAGAIN error message is 101 characters long (plus another 1 for the null
terminator) so we're off by 2 there. Perhaps a neater way to get these
error messages would be to use strerror() once we have the error number,
but for now Ill fix this array size bug and remove the redundant -1
check in the java code for now, and we can keep discussing the correct
behaviour here.
I don't know what the correct behavior is in this case, I just mentioned
it because it looked odd.
Understood - I think matching the RI here makes sense. I committed the
removal of the -1 check and the char array size bug at repo revision
r819983.
Regards,
Oliver
Regards,
Tim
--
Oliver Deakin
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
