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. >> 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 ;-) >> 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. Regards, Tim
