harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject Re: [classlib][luni] calling mmap
Date Tue, 29 Sep 2009 15:47:42 GMT
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

Mime
View raw message