harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oliver Deakin <oliver.dea...@googlemail.com>
Subject Re: [classlib][luni] calling mmap
Date Tue, 29 Sep 2009 15:57:37 GMT
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


Mime
View raw message